7

While I was writing a private helper method in Java, I needed to add a check for a precondition that would cause the method to do nothing if not met. The last few lines of the method were just off the bottom of the editing area in the IDE, so in an epic display of laziness, I wrote the check like this:

function(){
    if (precondition == false){
        return;
    }
    //Do stuff
}

as to opposed finding the end of the block to write a more "traditional" statement like this:

function(){
    if (precondition == true){
        //Do stuff
    }
}

This made me wonder: is there a reason to avoid my version in favor of the other, or is it just a stylistic difference? (Assuming they are used in such a way that they are meant to be functionally equivalent)

ThisIsNoZaku
  • 171
  • 1
  • 1
  • 8
  • 1
    And a lot more duplicates besides. Both ways are of course correct, but there is a lot of controversy over which is 'best'. My favorite answer is 'it depends'. [why-should-a-function-have-only-one-exit-point](http://stackoverflow.com/questions/4838828/), [Any reasearch on maintainability of guard statement vs. single exit point?](http://stackoverflow.com/questions/2292702) etc. – Patrick M Jul 22 '14 at 01:31

2 Answers2

24

Those are called Guard Clauses. They're actually desirable. They are one of several techniques that can be used to decompose complex nested if conditions into simpler (and easier to follow) logic.

As an example, this code:

public Foo merge (Foo a, Foo b) {
    if (a == null) return b;
    if (b == null) return a;
    // complicated merge code goes here.
  }

Replaces the more complicated, but having a single exit point:

public Foo merge (Foo a, Foo b) {
    Foo result;
    if (a != null) {
      if (b != null) {
        // complicated merge code goes here.
      } else {
        result = a;
      }
    } else {
      result = b;
    }
    return result;
  }

http://c2.com/cgi/wiki?GuardClause

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
  • 2
    This is the answer to the question I asked, but not the one I meant, but the one I meant doesn't make any sense on further consideration, so you win the prize. :) – ThisIsNoZaku Jul 21 '14 at 22:12
2

If you test for precondition equal false or true in Java, that means the precondition is a boolean value, so just use it directly:

if (precondition) {

if (!precondition) {

Skipping out of a function early is a nice way to keep code readable without a lot of nesting. But if you have a long list of preconditions, it might be hard to find the actual code, so it can be better to extract the real working code into an own method that gets called after the check succeeds:

void someFunction() {
    if (!precondition1) return;
    if (!precondition2) return;
    if (!precondition3) return;
    doStuff();
}

void doStuff() {
    //do stuff
}

For smaller checks the positive version should be preferred as it is easier to read.

user143530
  • 21
  • 1
  • 5
    I disagree extracting the doStuff() method, because if you call doStuff() directly there is no check on the preconditions at all. Sure you can prevent calling doStuff() directly, but there are still lots of scenarions where doStuff() could be called directly. – Simon Jul 22 '14 at 06:21