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;
}

Calmas

My rule of thumb: Return early if a guard condition fails, single point of return after the guard condition(s) unless it severely effects readability. – Lavernlaverna

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;
}

Carce

Given that it can be set to null, SomeType has to be some kind of pointer, so you probably mean SomeType result = new SomeTypeInternal();, where typedef SomeTypeInternal* SomeType;. That said, delaying construction until you actually need something is a very, very good idea. Too many people who started with C or Pascal forget that you can declare variables at any point in C++. – Dielectric

Yes, I intended to give the examples in pseudocode, but they ended up coming out as Java! – Calmas

Ah. I don't use Java, so I thought it was C++. Sorry. A 'Java' tag on the question would have helped. – Dielectric

@Mike: Actually, the proper tag would've been language-agnostic, the point being that the question wasn't about pointer semantics or similar language details, but instead about whether to return from a method/function early or late. – Alkaline

Language-agnostic observation: it doesn't matter (in general) where you declare the variable, it matters where you create objects. The initial example, with the variable declared before guardCondition() (and initialized to NULL!) is perfectly fine. If you have a local variable, chances are that your compiler will reserve stack space for it regardless on where exactly in the body of the function you are using that variable - so you don't save any memory by late declaration ( you may improve code readability though). – Cordero

@stakx: Actually, the proper tag would have been Java, the point being that the sample code was in Java and handling guard conditions in a different language may be radically different from any Java solution. – Persevering

@Cordero code readability is one of the most important programming issues, so it is a big mistake to say it doesn't matter when readability is affected – Carce

It didn't want to tag the question as Java because it's not a question about Java and I didn't want to limit the audience. Feel free to edit and substitute the code with pseudocode if you wish. – Calmas

@Tim: The syntax might be Java, but the question isn't about Java. It's about imperative programming in general. (It applies virtually immediately to C#, C and C++, and with some more syntactic changes to Python, Perl, Ruby, Tcl, ...) It's pretty strongly agnostic. – Louella

@Donal Fellows Thanks. Hopefully the example code should be understandable regardless of the reader's language background. – Calmas

Answer 2

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? ;)

Cruck

Wow, that brings back nightmares. Talk about ruling out the concept of exceptions. – Dielectric

Answer 3

Style 1 is what the Linux kernel indirectly recommends.

From https://www.kernel.org/doc/Documentation/process/coding-style.rst, 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.

Dielectric

Answer 4

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 ^^

Forwarding

Answer 5

Martin Fowler refers to this refactoring as : "Replace Nested Conditional with Guard Clauses"

If/else statements also brings cyclomatic complexity. Hence harder to test cases. In order to test all the if/else blocks you might need to input lots of options.

Where as if there are any guard clauses, you can test them first, and deal with the real logic inside the if/else clauses in a clearer fashion.

Stokehold

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.

Nutria

Answer 7

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.

Pellerin

In Java you should use finally to handle things such as closing filestreams. – Linebacker

Answer 8

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.

Deathwatch

Who says it "goes against best practices"? – Sind

Honestly I've only heard it from teachers. I guess it isn't really a best practice in the field. – Deathwatch

Answer 9

I would say that Style1 became more used because is the best practice if you combine it with small methods.

Style2 look a better solution when you have big methods. When you have them ... you have some common code that you want to execute no matter how you exit. But the proper solution is not to force a single exit point but to make the methods smaller.

For example if you want to extract a sequence of code from a big method, and this method has two exit points you start to have problems, is hard to do it automatically. When i have a big method written in style1 i usually transform it in style2, then i extract methods then in each of them i should have Style1 code.

So Style1 is best but is compatible with small methods. Style2 is not so good but is recommended if you have big methods that you don't want, have time to split.

Gibber

I can't really understand, why this is not the accepted answer. It is the only one so far addressing the problem with long and short functions. Everything else here is more an answer by personal preferences or lacking any arguments for both styles. – Naca

Answer 10

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.

Blakely

Answer 11

I would say "It depends on..."

In situations where I have to perform a cleanup sequence with more than 2 or 3 lines before leaving a function/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.

Watercress

Answer 12

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.

Isometric

Lazy and sloppy may be too strong of terms. Think of the guard conditions like assertions. If they aren't true there is no point continuing with the rest of the code in the method. That isn't lazy or sloppy. – Aila

Perhaps however it would be cleaner then if assertions were used. These return X statements are controversial because they result in more than 1 exit point from a function. Unfortunately, I've also seen developers going further to keep that single exit point in place while still using such guard conditions with goto exit instead in various languages - C, COBOL, Visual Basic. For example, read through the MySQL source and you'll see such goto some_exit_label occurances throughout the code. – Isometric

While I disagree with "lazy and sloppy", you make some good points about how the "style 2" quickly illuminates when a function is too complex. Certainly this answer doesn't deserve -4... you just pissed people off with "lazy and sloppy". Having said all that, style #1 is actually more correct in modern OO languages, because resource cleanup -- the primary bugaboo in C for style 1 -- becomes a non-issue with RAII. That leaves style 1 with the potential to dramatically reduce the number of logic paths. This has been shown to yield lower bug counts. – Muddler