Is using return at the start of method bad coding practice?

Question

I have found myself using the following practice, but something inside me kind of cringes every time i use it. Basically, it's a precondition test on the parameters to determine if the actual work should be done.

public static void doSomething(List<String> things)
{
    if(things == null || things.size() <= 0)
        return;

    //...snip... do actual work
}

Answer 1

It is good practice to return at the earliest opportunity.
That way the least amount of code gets executed and evaluated.

Code that does not run cannot be in error.

Furthermore it makes the function easier to read, because you do not have to deal with all the cases that do not apply anymore.

Compare the following code

private Date someMethod(Boolean test) {
  Date result;
  if (null == test) {
    result = null
  } else {
    result = test ? something : other;
  }
  return result;
} 

vs

private Date someMethod(Boolean test) {

  if (null == test) { 
    return null 
  }
  return test ? something : other;
} 

The second one is shorter, does not need an else and does not need the temp variable.

Note that in Java the return statement exits the function right away; in other languages (e.g. Pascal) the almost equivalent code result:= something; does not return.
Because of this fact it is customary to return at many points in Java methods.
Calling this bad practice is ignoring the fact that that particular train has long since left the station in Java.

If you are going to exit a function at many points in a function anyway, it's best to exit at the earliest opportunity

Answer 2

To the best of my understanding - no.

For the sake of easier debugging there should be only one return/exit point in a subroutine, method or function.

With such approach your program may become longer and less readable, but while debugging you can put a break point at the exit and always see the state of what you return. For example you can log the state of all local variables - it may be really helpful for troubleshooting.

It looks like there a two "schools" - one says "return as early as possible", whereas another one says "there should be only one return/exit point in a program".

I am a proponent of the first one, though in practice sometimes follow the second one, just to save time.

Also, do not forget about exceptions. Very often the fact that you have to return from a method early means that you are in an exceptional situation. In your example I think throwing an exception is more appropriate.

Answer 3

There's nothing inherently wrong with it, but if it makes you cringe, you could throw an IllegalArgumentException instead. In some cases, that's more accurate. It could, however, result in a bunch of code that look this whenever you call doSomething:

try {
    doSomething(myList);
} catch (IllegalArgumentException e) {}

Answer 4

It's a matter of style and personal preference. There's nothing wrong with it.