4
\$\begingroup\$

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<String, Object> paths)
  • A Collection of case paths. (Collection<String> casePaths)
  • An auxiliary Map as an alias os those paths (Map<String, String> choiceCaseAliases, being: "simplified path" ➔ "complete path")

I have the following ugly method:

private Map<String, String> resolveChoicePaths() {
    String path;
    for (String casePath: casePaths) {
        String choiceCase = casePath.replaceFirst(".*/([^/]+/)", "$1")+"/";
        boolean mapModified = false;
        Map<String, String> tmp = new LinkedHashMap<>();
        for (Iterator entries = choiceCaseAliases.entrySet().iterator(); entries.hasNext();) {
            Map.Entry<String, String> entry = (Map.Entry<String, String>) 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);
                    choiceCaseAliases.put(path, pathKey);
                }
            }
        } else {
            choiceCaseAliases.putAll(tmp);
        }
    }
    return choiceCaseAliases;
}

which is call here when paths map is completely populated:

Map<String, String> resolvedChoicePaths = resolveChoicePaths();
for (Map.Entry<String, String> entry: resolvedChoicePaths.entrySet()) {
    paths.getNodes().put(entry.getKey(), paths.getNodes().get(entry.getValue()));
    paths.getNodes().remove(entry.getValue());
}

Every thing works exactly as expected, but I think there must be a cleaner way to implement the same thing this method is doing. I think that this logic might be an overkill approach (that works, yay!) to the problem which actually is kind of complex, but I don't know if that much.

OBS:

  • Technically, I need to put and delete things at the same iteration on the same map, but since it's not possible AFAIK I have a temporary map for this.

  • That regex which is used to get a String choiceCase is bacause the parent statement of a case is always a Choice. So from a path /container/choice/case I can get the String: choice/case/ and search for it on the other elements complete path.

\$\endgroup\$
2
\$\begingroup\$

Just some points:

  1. 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.

  2. 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()) {
  1. 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<String, String> resolveChoicePaths() {
        String path;
        Map<String, String> 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<String, String> entry = (Map.Entry<String, String>) 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;
    }
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.