Replace LinkedHashMap by HashMap or Map #266
Conversation
| @@ -849,16 +848,16 @@ public void init(boolean forceNewConnection) { | |||
| it.remove(); | |||
| } | |||
|
|
|||
| ArrayList<LinkedHashMap<String, Object>> configInstances = | |||
| ((ArrayList<LinkedHashMap<String, Object>>) yamlConfig.getYamlInstances()); | |||
| ArrayList<Map<String, Object>> configInstances = | |||
There was a problem hiding this comment.
I know this is outside the scope of your changes, and is more a style thing, but since we are changing to the interface with Map maybe we should do the same with List?
There was a problem hiding this comment.
This is effectively outside of the scope and it will be addressed in a next PR :)
I think there are already enough changes for a PR 😅
There was a problem hiding this comment.
I didn't see anything wrong with this, I agree that order doesn't matter in general, not for instance ordering, not for config settings, not for bean collection order, etc. So no objections.
We do have some checkstyle errors apparently (I related to import lexicographical order from what I could see), so we should fix those before merging, but other than that, this looks good! Thank you for all the cleanup too! 🙇
Configurations come mostly as LinkedHashMap from Yaml or Json parsers But usage inside JMXFetch is mostly as HashMap. As LinkedHashMap is heavier regarding memory footprint, we prefer to avoid its usage by just replacing by HashMap. Also it does not seem That order matters in the case of JMXFetch usage. I have changed also the type to use Map interface as it will be easier To change the implementation if required in the future without changing this type everywhere.
jpbempel
force-pushed
the
jpbempel/replaceLinkedHashMap
branch
from
def2491 to
9405f32
Compare
Configurations come mostly as LinkedHashMap from Yaml or Json parsers
But usage inside JMXFetch is mostly as HashMap.
As LinkedHashMap is heavier regarding memory footprint, we prefer to
avoid its usage by just replacing by HashMap. Also it does not seem
That order matters in the case of JMXFetch usage.
I have changed also the type to use Map interface as it will be easier
to change the implementation if required in the future without
changing this type everywhere.