9

When the code flow is like this:

if(check())
{
  ...
  ...
  if(check())
  {
    ...
    ...
    if(check())
    {
      ...
      ...
    }
  }
}

I have generally seen this work around to avoid this messy code flow:

do {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(false);

What are some better ways that avoid this workaround/hack so that it becomes a higher-level (industry level) code?

Are there maybe constructs that come from Apache commons or Google Guava?

Note: this is a copy of the same question for C++. The best answers there are truly functions pointers and the GOTO command. Both doesn't exist in Java. I am eagerly interested in the same thing for Java.

Putting it into a new function and using return is in my opinion not a good solution, because return quits the method. So if my class has 20 methods with these constructs, I would have to add 20 additional methods to get this done. That's why GOTO was the best answer for C++.

Community
  • 1
  • 1
Kenyakorn Ketsombut
  • 2,072
  • 2
  • 26
  • 43
  • 6
    Some of the answers still hold - moving it into its own function, and using return for example. – Sinkingpoint Aug 30 '13 at 09:31
  • @MarounMaroun Because you can have this 20 times in a function to pass 20 serial steps. If I do it like this, the final if slides already 20 tabs to the right, which makes the code unreadable. – Kenyakorn Ketsombut Aug 30 '13 at 09:34
  • 3
    I am not a religious person, but I truly belief that "GOTO" is not the right answer for anything related to Java. – WarrenFaith Aug 30 '13 at 09:35
  • @Quirliom I have added a sentence to the question, why it is not good, unfortunately you were much too fast :) Which other answers still hold? I don't see any. – Kenyakorn Ketsombut Aug 30 '13 at 09:37
  • The `do` and `while (false);` parts are unnecessary - just use a plain block. (`{ if (!check()) break; }`) – Sam Marsh Aug 30 '13 at 09:41
  • @KenyakornKetsombut there was a support of GOTO in Java once but was removed as there are way better solutions (which I would count C++ to, as well): http://stackoverflow.com/a/4547764/180538 Beside the fact, that the GOTO answer on the linked post was noth "the best answer for c++" (at least if you consider upvotes) – WarrenFaith Aug 30 '13 at 09:46

4 Answers4

6

To reduce the cyclomatic complexity, you can separate the logics into submethods:

public void go() {
    first();
}

public void first() {
    if(check()) {
        // #1
        second();
    }
}

public void second() {
    if(check()) {
        // #2
        third();
    }
}

public void third() {
    if(check()) {
        // #3
    }
}
sp00m
  • 47,968
  • 31
  • 142
  • 252
  • Thanks for your reply. Yes, you are right about the complexity. Nevertheless, in this question the focus is more on readability and code-beauty. Speed is not the primary goal. I assume anyway that modern JIT compilers will handle that fine. – Kenyakorn Ketsombut Aug 30 '13 at 09:58
6

What is this "while(false)" nonsence? Use a label!

myLabel: {
    if(!check()) break myLabel;
    ...
    ...
    if(!check()) break myLabel;
    ...
    ...
    if(!check()) break myLabel;
    ...
}

It's core Java: http://docs.oracle.com/javase/tutorial/java/nutsandbolts/branch.html

mvmn
  • 3,717
  • 27
  • 30
  • 1
    I am accepting this, because it is significantly close to the original source. Of course the other answers are also pretty good. This one addresses exactly the problem that C++ (and Java) users have and obviously there is a very easy solution in Java. Thanks for this contribution. – Kenyakorn Ketsombut Sep 26 '13 at 08:40
2

Because you can have this 20 times in a function to pass 20 serial steps. If I do it like this, the final if slides already 20 tabs to the right, which makes the code unreadable.

This comment of yours shows the issue: you are imagine the wrong code structure.

For example your code is like this

if (bool) {
     // do something
     if (anotherBoolean) {
         //do even more
         if (thirdBoolean) {
             // here we go! I spare deeper structure...
         }
     }
}

This can be refactored easily to:

public void method() {
    if (bool) {
        doSomeStuff();
    }
}

public void doSomeStuff() {
    if (anotherBoolean) {
        doThirdStuff();
    }
}

public void doThirdStuff() {
    if (thirdBoolean) {
        doEvenMore();
    }
}

public void doEvenMore() {
    // deeper structure
}

That kind of code structure would be a good sample of Clean Code as the methods are doing their "single purpose" stuff, you don't do a while-loop hack plus you even might reuse the methods somewhere.

WarrenFaith
  • 57,492
  • 25
  • 134
  • 150
  • Thanks for this contribution. In my case I have 4 methods in my class and they run over 6 booleans. So I would have to add 24 methods to the class. Now try to find 24 different names for methods. Okay, I aggree, that depends on my actual code. Instead of finding more names, I could add parameters etc. So your solution might work for specific cases. I thought someone might have something cooler in mind like switch, exception-catches or static-imports. Anything else than creating new methods. – Kenyakorn Ketsombut Aug 30 '13 at 09:56
  • 1
    If your 6 booleans representing some kind of states, you could try enum and use them in a switch case statement. I use that for input handling, for example. State.PRESSED, RELEASE, MOVING, NONE etc and call methods according to that state. – WarrenFaith Aug 30 '13 at 10:02
1

It's a good question. The semantics of any solution must involve some kind of excitable block, either literal or implied by implementation that can be exited.

Some of the other answers are so complicated that they while they achieve avoiding the do-while loop, lose all clarity and readability, which makes them "bad" code.

After some thought, this is how I would code it:

doSomething(); // Replace current code with a method call

private void doSomething() {
    if(!check()) return;
    ...
    ...
    if(!check()) return;
    ...
    ...
    if(!check()) return;
    ...
    ...
}

Main points:

  • A method is a block, which is cleanly exited by return
  • The three parts clearly function as one programming unit, so together they are a string candidate for refactoring into a separate method
  • No "contrivances", such as the perfunctory do-while(false) construct, which exists only to provide an exitable block and which may confuse others
  • By using negative tests then exiting, two things are achieved:
    • the "exit early" coding paradigm
    • less indentation/nesting, which reduces the cyclic complexity
  • No extra blocks/classes/objects are created, maintaining readability and performance (although a frame will be pushed onto the stack for the method call - an negligible impact
Bohemian
  • 412,405
  • 93
  • 575
  • 722