Reputation: 2653

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

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
}

Upvotes: 10

Answers (11)

Wand Maker

Reputation: 18762

If function is long (say, 20 lines or more), then, it is good to return for few error conditions in the beginning so that reader of code can focus on logic when reading rest of the function. If function is small (say 5 lines or less), then return statements in the beginning can be distracting for reader.

So, decision should be based on primarily on whether the function becomes more readable or less readable.

Upvotes: 0

Alex Kreutznaer

Reputation: 1170

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.

Upvotes: 4

Johan

Reputation: 76641

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

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

Upvotes: 17

henry

Reputation: 6096

There is no correct answer to this question, it is a matter of taste.

In the specific example above there may be better ways of enforcing a pre-condition, but I view the general pattern of multiple early returns as akin to guards in functional programming.

I personally have no issue with this style - I think it can result in cleaner code. Trying contort everything to have a single exit point can increase verbosity and reduce readability.

Upvotes: 2

Guillaume M

Reputation: 510

If you want to avoid the "return" in your method : maybe you could use a subClass of Exception of your own and handle it in your method's call ?

For example :

public static void doSomething(List<String> things) throws MyExceptionIfThingsIsEmpty {
    if(things == null || things.size() <= 0)
        throw new MyExceptionIfThingsIsEmpty(1, "Error, the list is empty !");    

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

Edit : If you don't want to use the "return" statement, you could do the opposite in the if() :

if(things != null && things.size() > 0)
// do your things

Upvotes: 0

raptortech97

Reputation: 507

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) {}

Upvotes: 2

Nova Entropy

Reputation: 5767

PMD seems to think so, and that you should always let your methods run to the end, however, for certain quick sanity checks, I still use premature return statements.

It does impair the readability of the method a little, but in some cases that can be better than adding yet another if statement or other means by which to run the method to the end for all cases.

Upvotes: 2

Jorge Antonio Diaz-Benito

Reputation: 1066

Java good practices say that, as often as possible, return statements should be unique and written at the end of the method. To control what you return, use a variable. However, for returning from a void method, like the example you use, what I'd do would be perform the check in a middle method used only for such purpose. Anyway, don't take this too serious - keywords like continue should never be used according to Java good practices, but they're there, inside your scope.

Upvotes: -1