How can I change my for loop full of if statements into more elegant/efficient code?

Viewed 132

This is my string:

String field = "first=true, second=true"

My method operates on this string and recognises if it contains the first= and second= substrings, and if yes - based on the true/false following it calls other methods. The first and second substrings may be optional though. This is what I have so far:

void method(String field) {

      String[] splittedField = field.split(", ");
      for (String substring : splittedField) {
          if (substring.contains("first") {
              if (substring.contains("true") {
                  otherMethod("first", "true");
              } else if (substring.contains("false") {
                  otherMethod("first", "false");
              }
          } else if (substring.contains("second") {
              if (substring.contains("true") {
                  otherMethod("second", "true");
              } else if (substring.contains("false") {
                  otherMethod("second", "false");
              }
          }
      }

}

But perhaps there is a better/more efficient(and elegant?) way of solving that case?

7 Answers

Consider:

if (substring.contains("first") {
    if (substring.contains("true") {
        otherMethod("first", "true");
    } else if (substring.contains("false") {
        otherMethod("first", "false");
    }
 } 

Above if can be coded as:

if (substring.contains("first") {
    String[] valueString = substring.split("=");            
    otherMethod("first", valueString[1]);
 }

You can do it simply as follows:

String[] splittedField = field.split(", ");
for (String substring : splittedField) {
    String[] parts = substring.split("=");
    otherMethod(parts[0], parts[1]);
}

You don't need all the if statements.

For the checks, you could create a couple of sets, and for invoking the method, you could split each substring again, by =:

void method(String field) {

    Set<String> firstSecond = Set.of("first", "second");
    Set<String> trueFalse = Set.of("true", "false");

    String[] splittedField = field.split(", ");
    for (String substring : splittedField) {
        String[] args = substring.split("=");
        if (firstSecond.contains(args[0]) && trueFalse.contains(args[1])) {
            otherMethod(args[0], args[1]);
        }
    }
}

Note: if you aren't using Java9+ yet, you could rewrite Set.of as follows:

Set<String> firstSecond = new HashSet<>(Arrays.asList("first", "second"));

I think something like this will eliminate the complex if structure.

public class Main {
    public static void method(String field) {
        int i = 0;
        // Make the flags false by default, 
        // in case you don't supply either or both of them in field.
        String[] flags = {"false", "false"};
        String[] splittedField = field.split(", ");
        for (String substring : splittedField) {
            String[] args = substring.split("=");
            flags[i] = args[1];
            ++i;
        }
        othermethod(flags[0], flags[1]);
    }

    public static void main(String[] args) {
            String field = "first=true, second=false";
            method(field);
    }
}

In a solution that uses Streams, you can perform something like:

void invokeOtherMethod(String field) {
    Map<String, Boolean> mapFromString = Arrays.stream(field.split(","))
            .map(s -> s.trim().split("="))
            .collect(Collectors.toMap(a -> a[0], a -> Boolean.valueOf(a[1])));
    for (Map.Entry<String, Boolean> entry : mapFromString.entrySet()) {
        otherMethod(entry.getKey(), entry.getValue());
    }
}

Note: The change in the signature of otherMethod on purpose to use:

void otherMethod(String key, boolean value)

Based on OP's question and his comment, this is enough I think?:

void method(String field) {

      Arrays.stream(field.split(", "))
            .forEach(v -> {
                 String[] args = v.split("=");
                 otherMethod(args[0], args[1]);
            })
}

Here is an improved version for your code. Some of the point that you should keep in mind. 1. You can directly call the contains method in the otherMethod as it is also expecting boolean.

void method(String field) {
  String[] splittedField = field.split(", ");
  for (String substring : splittedField) {
    if (substring.contains("first") {
        otherMethod("first", substring.contains("true"));
    } else if (substring.contains("second") {
        otherMethod("second", substring.contains("true"));
    }
  }
}
Related