Programming style: should you return early if a guard condition is not satisfied?

Question

One thing I've sometimes wondered is which is the better style out of the two shown below (if any)? Is it better to return immediately if a guard condition hasn't been satisfied, or should you only do the other stuff if the guard condition is satisfied?

For the sake of argument, please assume that the guard condition is a simple test that returns a boolean, such as checking to see if an element is in a collection, rather than something that might affect the control flow by throwing an exception. Also assume that methods/functions are short enough not to require editor scrolling.

// Style 1
public SomeType aMethod() {
  SomeType result = null;

  if (!guardCondition()) {
    return result;
  }

  doStuffToResult(result);
  doMoreStuffToResult(result);

  return result;
}

// Style 2
public SomeType aMethod() {
  SomeType result = null;

  if (guardCondition()) {
    doStuffToResult(result);
    doMoreStuffToResult(result);
  }

  return result;
}

Answer 1

I prefer the first style, except that I wouldn't create a variable when there is no need for it. I'd do this:

// Style 3
public SomeType aMethod() {

  if (!guardCondition()) {
    return null;
  }

  SomeType result = new SomeType();
  doStuffToResult(result);
  doMoreStuffToResult(result);

  return result;
}

Answer 2

Although it goes against best practices that I have been taught I find it much better to reduce the nesting of if statements when I have a condition such as this. I think it is much easier to read and although it exits in more than one place it is still very easy to debug.

Answer 3

I don't know if guard is the right word here. Normally an unsatisfied guard results in an exception or assertion.
But beside this I'd go with style 1, because it keeps the code cleaner in my opinion. You have a simple example with only one condition. But what happens with many conditions and style 2? It leads to a lot of nested ifs or huge if-conditions (with || , &&). I think it is better to return from a method as soon as you know that you can.
But this is certainly very subjective ^^

Answer 4

Style 1 is what the Linux kernel indirectly recommends.

From http://www.kernel.org/doc/Documentation/CodingStyle, chapter 1:

Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program.

Style 2 adds levels of indentation, ergo, it is discouraged.

Personally, I like style 1 as well. Style 2 makes it harder to match up closing braces in functions that have several guard tests.

Answer 5

It sometimes depends on the language and what kinds of "resources" that you are using (e.g. open file handles).

In C, Style 2 is definitely safer and more convenient because a function has to close and/or release any resources that it obtained during execution. This includes allocated memory blocks, file handles, handles to operating system resources such as threads or drawing contexts, locks on mutexes, and any number of other things. Delaying the return until the very end or otherwise restricting the number of exits from a function allows the programmer to more easily ensure that s/he properly cleans up, helping to prevent memory leaks, handle leaks, deadlock, and other problems.

In C++ using RAII-style programming, both styles are equally safe, so you can pick one that is more convenient. Personally I use Style 1 with RAII-style C++. C++ without RAII is like C, so, again, Style 2 is probably better in that case.

In languages like Java with garbage collection, the runtime helps smooth over the differences between the two styles because it cleans up after itself. However, there can be subtle issues with these languages, too, if you don't explicitly "close" some types of objects. For example, if you construct a new java.io.FileOutputStream and do not close it before returning, then the associated operating system handle will remain open until the runtime garbage collects the FileOutputStream instance that has fallen out of scope. This could mean that another process or thread that needs to open the file for writing may be unable to until the FileOutputStream instance is collected.

Answer 6

If you dig through the .net-Framework using .net-Reflector you will see the .net programmers use style 1 (or maybe style 3 already mentioned by unbeli). The reasons are already mentioned by the answers above. and maybe one other reason is to make the code better readable, concise and clear. the most thing this style is used is when checking the input parameters, you always have to do this if you program a kind of frawework/library/dll. first check all input parameters than work with them.

Answer 7

I prefer to use method #1 myself, it is logically easier to read and also logically more similar to what we are trying to do. (if something bad happens, exit function NOW, do not pass go, do not collect $200)

Furthermore, most of the time you would want to return a value that is not a logically possible result (ie -1) to indicate to the user who called the function that the function failed to execute properly and to take appropriate action. This lends itself better to method #1 as well.

Answer 8

Having been trained in Jackson Structured Programming in the late '80s, my ingrained philosophy was always "a function should have a single entry-point and a single exit-point"; this meant I wrote code according to Style 2.

In the last few years I have come to realise that code written in this style is often overcomplex and hard to read/maintain, and I have switched to Style 1.

Who says old dogs can't learn new tricks? ;)

Answer 9

I would say "It depends on..."

In situaltions where I have to perform a cleanup sequence with more than 2 or 3 lines before leaving a funtion/method I would prefer style 2 because the cleanup sequence has to be written and modified only once. That means maintainability is easier.

In all other cases I would prefer style 1.

Answer 10

Number 1 is typically the easy, lazy and sloppy way. Number 2 expresses the logic cleanly. What others have pointed out is that yes it can become cumbersome. This tendency though has an important benefit. Style #1 can hide that your function is probably doing too much. It doesn't visually demonstrate the complexity of what's going on very well. I.e. it prevents the code from saying to you "hey this is getting a bit too complex for this one function". It also makes it a bit easier for other developers that don't know your code to miss those returns sprinkled here and there, at first glance anyway.

So let the code speak. When you see long conditions appearing or nested if statements it is saying that maybe it would be better to break this stuff up into multiple functions or that it needs to be rewritten more elegantly.