-2

Need help in optimzing this java code.The cyclomatic complexity is 20 need to be 15.

private static void replaceNodeProperties(HashMap configNodeProperties, JSONArray esdNodepropertiesList) 
{

    HashMap replaceConfigNodePropRules = (HashMap) configNodeProperties.get("replace");

    if (replaceConfigNodePropRules != null) {
      for (Object replaceConfigNodeProp : replaceConfigNodePropRules.keySet()) {
        for (int j = 0; j < esdNodepropertiesList.size(); j++) {
          JSONObject esdNodePropObj = (JSONObject) esdNodepropertiesList.get(j);
          if (esdNodePropObj.get("name").toString().contains("::")) {
            esdNodePropObj.put("name", esdNodePropObj.get("name").toString().split("::")[1]);
          }
          if (esdNodePropObj.get("name").equals(replaceConfigNodeProp)) {
            if (replaceConfigNodeProp.equals("resourceType")) {
              esdNodepropertiesList.remove(j);
              ArrayList replaceNodePropList = (ArrayList) replaceConfigNodePropRules.get(replaceConfigNodeProp);
              esdNodepropertiesList.addAll(replaceNodePropList);
            } else if (replaceConfigNodeProp.equals("comments"))
              esdNodePropObj.put("name", replaceConfigNodePropRules.get(replaceConfigNodeProp));
            break;
          }
        }
      }
    }
  }
VLAZ
  • 26,331
  • 9
  • 49
  • 67
  • 1
    What cyclomatic complexity algorithm do you use (e.g. McCabe, Prater)? – jubnzv Mar 16 '21 at 07:41
  • Do you need to reduce its complexity or is your IDE just flagging this as complex? I think the first think I would attempt to do is extract the inner `for` loop to its own method. – Gavin Mar 16 '21 at 09:08
  • Sonar in my IDE is flagging it as complex. Able to resolve this by making 2 small methods for the for loops. – Kautilya Mar 16 '21 at 11:21

1 Answers1

0

There are several ways to refactor to reduce the cyclomatic complexity of the code. Regardless of the estimation method used, the key idea is to reduce the number of linearly independent paths through the code.

In general, you can achieve it:

  • By using the early return: exit of the method when a condition allows to do it and not waiting for a single exit point.
  • Avoiding not required else statements.
  • Avoiding nested loops. In some cases, you can create a new method that implements the required functionality instead.
  • Using smaller methods in general. This not only reduces the cyclomatic complexity of the code, but also makes it easier to test your code.

In your particular case I suggest you:

  1. Exit early when replaceConfigNodePropRules == null.
  2. Try to create a smaller methods to process the nested configNode objects instead of using loops.

Finally, keep in mind that this is not always possible to reduce the complexity. Here are some functions which naturally have to be a bit complicated - validating user input or parsing are examples.

jubnzv
  • 1,526
  • 2
  • 8
  • 20