How to satisfy SonarLint java:S135 rule and keep code clean?

Question

SonarLint's rule java:S135 says:

Loops should not contain more than a single "break" or "continue" statement The use of break and continue statements increases the complexity of the control flow and makes it harder to understand the program logic. In order to keep a good program structure, they should not be applied more than once per loop. This rule reports an issue when there is more than one break or continue statement in a loop. The code should be refactored to increase readability if there is more than one.

And gives this example:

for (int i = 1; i <= 10; i++) {     // Noncompliant; two "continue" statements
  if (i % 2 == 0) {
    continue;
  }

  if (i % 3 == 0) {
    continue;
  }
  // ...
}

Should be refactored to:

for (int i = 1; i <= 10; i++) {
  if (i % 2 == 0 || i % 3 == 0) {
    continue;
  }
  // ...
}

But it looks weird. I have this test example:

@Slf4j
public class TestService {

    public int test(List<String> input) {
        int count = 0;
        for (String s : input) {
            if ("ONE".equals(s)) {
                log.info("one was called");
                processOne(s);
                continue;
            }

            String something = getSomethingBy(s);
            if (something == null) {
                log.info("something is null");
                sendEmail();
                continue;
            }

            if ("TWO".equals(s) && "hz".equals(something)) {
                process(s);
                count++;
            }
        }
        return count;
    }

and sonarLint claims:

If I change this code to:

public int testGoodForSonarLint(List<String> input) {
        int count = 0;
        for (String s : input) {
            if ("ONE".equals(s)) {
                log.info("one was called");
                processOne(s);
            } else {
                String something = getSomethingBy(s);
                if (something == null) {
                    log.info("something is null");
                    sendEmail();
                } else if ("TWO".equals(s) && "hz".equals(something)) {
                    process(s);
                    count++;
                }
            }
        }
        return count;
    }

It looks so bad. I hate that if-else chin and hell of {}. The code looks like a Christmas tree. And it is with 2 continue. In real code, I have 4. So, it is not always possible to refactor as SonarLint suggests:

if (i % 2 == 0 || i % 3 == 0) {
        continue;
      }

Because I need to do something in each condition, not just continue.

Maybe I don't understand something but interapting cycle by continue or break looks much better and readable than brackets hell. What do you think, is it possible to rewrite this code? if yes, how would you refactor my example?

Answer 1

You could create another method for an iteration and return in it.

Then, it would not be jumping around within the method but only stopping execution of the method.

@Slf4j
public class TestService {

    public int test(List<String> input) {
        int count = 0;
        for (String s : input) {
             count = processString(s, count);
        }
        return count;
    }
    private int processString(String s, int count){
        if ("ONE".equals(s)) {
            log.info("one was called");
            processOne(s);
            return count;
        }

        String something = getSomethingBy(s);
        if (something == null) {
            log.info("something is null");
            sendEmail();
            return count;
        }

        if ("TWO".equals(s) && "hz".equals(something)) {
            process(s);
            count++;
        }
        return count;
    }
}

or even better, don't pass count as a parameter:

@Slf4j
public class TestService {

    public int test(List<String> input) {
        int count = 0;
        for (String s : input) {
             count += processString(s);
        }
        return count;
    }
    private int processString(String s){
        if ("ONE".equals(s)) {
            log.info("one was called");
            processOne(s);
            return 0;
        }

        String something = getSomethingBy(s);
        if (something == null) {
            log.info("something is null");
            sendEmail();
            return 0;
        }

        if ("TWO".equals(s) && "hz".equals(something)) {
            process(s);
            return 1;
        }
        return 0;
    }
}

(you could also return a boolean and increment count iff it is true)

But at the end, whether or not you like this code is your choice. This way doesn't eliminate exiting out of the if but it gets rid of going to other positions within the current method which can be confusing.