Optimizing "complex" iteration over Map with auxiliary maps

This will be tough to explain. I am implementing a tool to support RFC6020. But when it comes to the Choice Statement, things get weird.

For the sake of better understanding, I have to remember you a couple of things:

• I have a XML and a YANG file according to the RFC.

• A Choice (and it's cases) are not included on the XML, only the children of the selected case. But the .yang instead, contains a full specification of the Choice and it's cases.

Based on this there is a moment where I have only read the .yang files and send a response to something that has only knowledge of the XML structure, thus no choices.

Basically my main problem is that this (complete) path on a .yang file:

/container/choice/case/leaf

would be simplified like this on an XPATH notation of the .xml file:

/container/leaf

Here is the basic logic of this task:

Consider that I have:

• A Map with all the elements and their complete path. (Map paths)

• A Collection of case paths. (Collection casePaths)

• An auxiliary Map as an alias os those paths

(Map choiceCaseAliases, being: "simplified path" ➔ "complete path")

I have the following ugly method:

```
private Map resolveChoicePaths() {
String path;
for (String casePath: casePaths) {
String choiceCase = casePath.replaceFirst(".*/([^/]+/)", "$1")+"/";
boolean mapModified = false;
Map tmp = new LinkedHashMap<>();
for (Iterator entries = choiceCaseAliases.entrySet().iterator(); entries.hasNext();) {
Map.Entry entry = (Map.Entry) entries.next();
if (entry.getKey().contains(choiceCase)) {
path = entry.getKey().replace(choiceCase, StringUtils.EMPTY);
tmp.put(path, entry.getValue());
entries.remove();
mapModified = true;
}
}
if (!mapModified) {
for (String pathKey: paths.keySet()) {
if (pathKey.contains(choiceCase)) {
path =

Just some points:

-
I don't see the initialization of choiceCaseAliases,casePaths, or entry. If they are class variables, that's fine, as long as it is meant to be a class variable. If they are outside the method just so that another method can set the values and have the current method run, then it is not okay. If they are class variable just for the method, have them as method arguments.

-
Here:

for (String casePath: casePaths) {

and here:

for (String pathKey: paths.keySet()) {

You don't follow Java conventions. There should be a space before and after the ::

for (String casePath : casePaths) {

and:

for (String pathKey : paths.keySet()) {

-
I wouldn't create a new temporary map for each loop. Instead, I would create one outside the loop, and dump the contents of the temporary map into the map at the end:

private Map resolveChoicePaths() {
    String path;
    Map tmp = new LinkedHashMap<>();
    for (String casePath : casePaths) {
        String choiceCase = casePath.replaceFirst(".*/([^/]+/)", "$1") + "/";
        boolean mapModified = false;
        for (Iterator entries = choiceCaseAliases.entrySet().iterator(); entries.hasNext();) {
            Map.Entry entry = (Map.Entry) entries.next();
            if (entry.getKey().contains(choiceCase)) {
                path = entry.getKey().replace(choiceCase, StringUtils.EMPTY);
                tmp.put(path, entry.getValue());
                entries.remove();
                mapModified = true;
            }
        }
        if (!mapModified) {
            for (String pathKey : paths.keySet()) {
                if (pathKey.contains(choiceCase)) {
                    path = pathKey.replace(choiceCase, StringUtils.EMPTY);
                    tmp.put(path, pathKey);
                }
            }
        }
    }
    choiceCaseAliases.putAll(tmp);
    return choiceCaseAliases;
}

StackExchange Code Review Q#110030, answer score: 2