views:

887

answers:

12

I'm doing some coding in JavaScript, and I am having a lot of instances where I have to check some stuff before I proceed. I got into the habit of returning early in the function, but I'm not sure if I am doing this right. I am not sure if it have an impact on the complexity of my code as it grows.

I want to know from more experienced JavaScript coders, what is a better general practice out of the following two examples. Or is it irrelevant, and they are both OK ways of writing this particular IF block?

1) Returning Early or "Short Circuit" as I call it (Guard Clause).

ServeAlcohol = function(age)
{
     if(age < 19)
         return;


     //...Code here for serving alcohol.....
}

..Or...

2) Wrap code into an IF statement.

ServeAlcohol = function(age)
{
     if(age >= 19)
     {
          //...Code here for serving alcohol.....
     }
}
+8  A: 

Hello,

I prefer the first one, because it's a guard condition and you exit directly. But I don't think there is performance issues with either, just that it's your preference... Both will return execution directly...

Brian
+8  A: 

Personal choice. For me, if there are some "stop" conditions I can check at the beginning of the method, I prefer using the "return" pattern. But only if I can do them all in the beginning of the method.

Danail
+46  A: 

Usually I have input-validation return right away. Imaging if you had a bunch of conditionals, you get a mess of nested ifs right away.

Generally once I get past input validation I avoid multiple returns, but for validation I return right away. Keeps it cleaner IMHO.

Parrots
+1 that is a good point about huge nested IF blocks.
Roberto Sebestyen
Generally its just a style of programming, But as the answer says, it will not be obvious for the guy who is going to maintain the code if the IF block is big.
Teja Kantamneni
+1 Generally its best for a block of code to have only one way in and one way out. There are some common sense exceptions, upfront validations would be one of them.
AnthonyWJones
@AnthonyWJones, Why is it best to have only one way in and one way out?
tster
@tster Multiple returns from a single method/function can cause headaches when memory needs to be deallocated or things like pass-by-reference variables need to get set before exiting. It also, in my opinion, makes the code much less readable, and more difficult to debug ("Why is my breakpoint not getting hit?? Oh, I returned from the function before execution made it there"). That being said, I also agree with a lot of this post: http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement for cases such as these.
Kevin
@Kevin: The memory-deallocation is no problem in Java, as the garbage-collector does that for you. So with manual memory management that is a strong argument for only one return-point. But in Java ... Less readable is questionable and depends strongly on the situation. To avoid nested ifs I would prefer guardians. If only one condition is checked an if should be better.
Mnementh
Yep. Return statements are to be expected within the first 10 and last 10 lines of a function - validation of input and validation of result (in case I return magic numbers to signal error)
SF.
+3  A: 

It really depends. I do like one point of return for simple functions, but anything longer than 10-20 lines and I'll start breaking things up for the sake of code clarity.

MauriceL
+2  A: 

I prefer the first one, because it's the process of elimination, where you return out of the function before the program even has to step through the next round of logic. I call it my prereq check - where the function won't execute if it doesn't meet the prereq check

In fact, I do this all the time, for example, the classic one is where i have a function that's expecting an integer and i get a string, i check at the top of the function if it's an integer, NOT if it's not a string or not another object/type, that's just stupid in my book.

It's like a college application to Harvard, a prerequisite:

'I don't want to even want you to come for an interview if you don't have a 3.5GPA or higher!'

:=)

Jacob Relkin
+5  A: 

FWIW, I'll offer a contrary opinion. Structured Programming suggests that a function should have one point of exit. I think there are some compiler optimizations that are not available if you use early returns, break statements, goto statements and the like. Also more branches in your code means its harder to fill the CPU pipeline resulting in a possible performance reduction... There are also reasons for not returning early that deal with rigorous (i.e. algebreic) reasoning about correctness.

Structured Programming wiki article

vicatcu
-1. Most code trying hard to follow that paradigm tends to be a multi-indent unreadable mess croaking 'kill me'... argh. Basically, your point is fine but it doesn't apply to 99.9% of all cases. Premature-optimization and all that. (And if your validation is a hot-spot, you have other problems. ;)
Marcus Lindblom
I'm very skeptical of these performance arguments. Code with early returns does not have more branches. It will likely compile to exactly the same code. The correctness argument is also dubious. Exceptions break the single-return assumption wide open--and languages these days have features like `finally`, `using` in C#, `with` in Python, and the RAII pattern in C++, that say "this code will run no matter what" explicitly and work even in the face of exceptions or early return.
Jason Orendorff
vicatcu is correct regarding the correctness argument: single-exit code is much easier to formally reason about(one reason why Dijkstra sought it). He's also correct about the pipeline flush problems. Generally I've found that carefully constructing a setup with single-exit code tends to be the most maintainable in the long run.
Paul Nathan
+2  A: 

The first one is usually preferred simply because it reduces the needed indentation (which could get way out of hand). There is no real performance difference.

BlueRaja - Danny Pflughoeft
A: 

In my opinion, as a best practice, I think it is more important to consistently use braces with your control blocks, even if their body is only one line.

Consistent

if ( condition ) {
    statement;
    statement;
}

if ( condition ) {
    statement;
}

Not consistent

if ( condition ) {
    statement;
    statement;
}

if ( condition )
    statement;

But even still, this is completely subjective.

As for when to break out of a function, and levels of indentation, that's subjective too. Research and experience have shown that exiting a function at only one point (the end) is easier to debug, optimize, etc. On the other hand, multiple levels of indentation can make a function difficult to read.

Justin Johnson
reference to that research?
Emile Vrijdags
+2  A: 

There are some people who think that each function should have a single exit point. However, I find it clearer when quick conditional checks like the one you mentioned are done at the beginning. It also avoid some code from being unnecessarily run.

+2  A: 

A general rule I've heard is basically fail early and fail often. You never know when a single line of code is pointing to some super-overloaded setter that's working way harder than you might think. If you can prevent that line of code from being executed - say, by returning early - then your code is going to be exponentially more efficient.

In other words, if you can return early and keep code from executing, do it at every turn - especially if you are concerned at all about performance. This might not be as important in something like JS, I suppose - I'm more of an AS3 guy - but the same logic applies.

If you have a lot of cases, it might be best also to trace out the point of failure in each one - in your example, trace out that this returned early because the age was too low. That'll help other developers who go in and attempt to debug your code, they'll know where things fail and why.

Hope that helps! :)

Myk
+1  A: 

Alternatively, since JavaScript is scheme in disguise

HandleRequestForAlcohol = function(age) { 
    ( IsUnderAge(age) ? RespondUnderAge : ServeAlcohol )();
}

The idiom for selecting the function isn't that important, rather that if you are doing complex validation and then have multiple processes, factor these to separate functions rather than making one big one, unless it's in a very performance critical bit of code.

Pete Kirkham
A: 

If there are multiple / complex guards, I would use the return. Otherwise in the case of one simple condition (in a smallish function) then I prefer using an if.

trinithis