Use while loop to avoid deeply nested if statements in java

Question

Hi I have written a little function like

public void foo(MyClassA paraA) {
    if (paraA == null) return;
    MyClassB paraB = doSomeStuff(paraA);
    if (paraB == null) return;
    MyClassC paraC = doMoreStuff(paraB);
    if (paraC == null) return;
    ....
}

The above fails fast and is nice to read (i.e. the intention to return on null values is clear). But now instead of simply returning, I want to do some error logging, so I changed to

public void foo(MyClassA paraA) {
    if (paraA == null) {doLog(); return;}
    MyClassB paraB = doSomeStuff(paraA);
    if (paraB == null) {doLog(); return;}
    MyClassC paraC = doMoreStuff(paraB);
    if (paraC == null) {doLog(); return;}
    ....
}

The above is also clean and easy to read, but I have to repeat doLog() a couple of times. So I change again to

public void foo(MyClassA paraA) {
    if (paraA != null) {
        MyClassB paraB = doSomeStuff(paraA);
        if (paraB != null) {
            MyClassC paraC = doMoreStuff(paraB);
            if (paraC != null) {
                ....
                return;
            }
        }
    }
    doLog();
}

The above calls doLog() only just once but I ended with some deeply-nested if statements, which are very ugly and hard to read. So how do I keep the same cleanliness like before and have doLog() just once? Note that returning something else rather than void for foo() is not allowed. And I also read that using try/catch as oppose to null check is an anti-pattern.

If I am to try, I want to write something like

public void foo(MyClassA paraA) {
    while(true) {
        if (paraA == null) break;
        MyClassB paraB = doSomeStuff(paraA);
        if (paraB == null) break;
        MyClassC paraC = doMoreStuff(paraB);
        if (paraC == null) break;
        ....
        return;
    }
    doLog();
}

The above fulfills all my needs(fail fast, clean, no nested if), but is the use of the while loop here an anti-pattern as the while loop here is never meant to run more than once?

@shree.pat18 havent thought about that far, why? May be you can give your suggestions on both with parameter or without, thank you. — user1589188, Feb 13 '15 at 07:07
If you can append to a string each time one of the variables is null, and then pass the resulting string to `doLog` once at the end, you wouldn't have to make repeated calls to it. — shree.pat18, Feb 13 '15 at 07:08
@shree.pat18 yes, that can be done but not related to my question. Which pattern you are talking about which calls doLog just once? If you are referring to the nested-if, I am against that. If you are referring to my while loop version, I have my question asking if using while loop an anti-pattern. — user1589188, Feb 13 '15 at 07:12
because of the return; it will not reach to dolog() method hope that is fine with you. — someone, Feb 13 '15 at 07:13
I think you could add the `paraA, paraB, paraN` to an array. Then iterate over the array (check for null). If its null, then create the instance of the value at `index[n]` and pass it in the valid params. — benscabbia, Feb 13 '15 at 07:14

David Ehrmann · Accepted Answer · 2015-02-13T07:16:45.757

3

Java has a nifty labeled break construct that might help you, here.

public void foo(MyClassA paraA) {
    block: {
        if (paraA == null) { break block; }
        MyClassB paraB = doSomeStuff(paraA);
        if (paraB == null) { break block; }
        MyClassC paraC = doMoreStuff(paraB);
        if (paraC == null) { break block; }
        ...
        return;
    }

    doLog();
}

If you used polymorphism better, you could do this:

public void foo(MyInterface para) {
    while (para != null) {
        para = para.doStuff();
    }
    doLog();
}

If you absolutely can't use polymorphism like that, use a dispatcher.

But I've seen this before, and it looks like a state machine. Give "java enum state machine" a search. I have a feeling that's what you're really trying to do.

edited Feb 13 '15 at 07:16

answered Feb 13 '15 at 07:11

David Ehrmann

7,366
2
31
40

I dont know you can use break without being inside a for or while loop! Thats interesting, let me try this. If it works, most likely I will pick yours as the answer. – user1589188 Feb 13 '15 at 07:18
See [JLS #14.15](http://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.15). – user207421 Feb 13 '15 at 07:20
@user1589188 Thanks, but please think about the polymorphic approach for this, even if you don't end up using it. It's a good pattern to know. – David Ehrmann Feb 13 '15 at 07:27
Yup, tried and it works. The only remaining question is, I feel this labelling sort of like goto. Isn't that another anti-pattern? Can't do poly-morphing doStuff() as I need to pass in some parameters at different stage. – user1589188 Feb 13 '15 at 07:31
It's not really any different from multiple returns from a single function...which is a bit like goto, but the abuses of goto were so much worse than multiple returns, break, and continue ever were. There are definitely people who don't like multiple returns, but I'd say it's really more of a personal preference at this point. – David Ehrmann Feb 13 '15 at 07:44
What if you did a state machine with a mutable Context that's modified on each iteration? – David Ehrmann Feb 13 '15 at 07:45

score 1 · Answer 2 · answered Feb 13 '15 at 07:08

1

Do you think that this is clean

public void foo(MyClassA paraA) {

    MyClassB paraB = paraA != null?doSomeStuff(paraA):null;
    MyClassC paraC = paraB != null?doMoreStuff(paraB):null;

     if (paraC != null) {
         ....

     }

     doLog();
}

answered Feb 13 '15 at 07:08

someone

6,577
7
37
60

Even better, put the null check in `doMoreStuff` – Cole Tobin Feb 13 '15 at 07:13
This is clean, but does not fail fast. Once you know paraA is null, you should go to doLog() directly, so yours is not optimal where you have to eval some more condition checks before reaching doLog() – user1589188 Feb 13 '15 at 07:15
1

null checks are as fast as it gets. Leave it to hotspot to optimize them with a jmp. – Has QUIT--Anony-Mousse Feb 14 '15 at 10:27

Has QUIT--Anony-Mousse · Answer 3 · 2015-02-14T10:57:01.087

IMHO your second code snippet is what ypu should do.

Do not try to make your code short. This is an antipattern.

if (a==null) {
  log("Failed in step a");
  return;
}
B b = a.doSomething();

is very fast to read and understand. You save nothing by compressing this code. Zero. Nada. Leave it to Hotspot VM, and focus on making code understandable. "if null then log return" is a classic, well understood and accepted pattern.

It had become popular to try to make code "readable" with lambda antipatterns like this:

B b = ifNullLog(a, () -> a.doSomething())

where

T ifNullLog(Object guard, Function<T> func) {
  if (guard == null) { doLog(); return null; }
  return func.run();
}

but IMHO this is a total antipattern. In fact, it is a best practise to even require braces for every if, else, for, while to make it easy to insert such a log statement without risking to break the code.

Code like your first snippet:

if (a == null) return;

are dangerous. Look at various bugs like Apples SSL disaster If someone adds doLog without noticing the missing brackets, the function will always return null. The apple SSL bug (or was it heartbleed?) esdentially was a

if (a==null)
  return;
  return;
B b = a.doSomething();

See how subtle the bug is? You Java compiler will fortunately warn you if it's about unreachable code - it will not necessarily warn you otherwise... by always using brackets and well formatted code such bugs can easily be avoided. Format code to avoid errors, not for aestetics.

It is also well acceptable to use return codes. Just don't make success default (see heartbleed again).

Code c = execute(a);
if (c != Code.SUCCESS) {
  doLog(c);
  return;
}

where

Code execute(A a) {
  if (a == null) { return Code.FAILED_A_NULL; }
  B b = a.doSomething();
  if (b == null) { return Code.FAILED_B_NULL; }
  ...
  return Code.SUCCESS;
}

A classic use case of "return", another good pattern.

Use while loop to avoid deeply nested if statements in java

3 Answers3