Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Possible Duplicate:
Elegant ways to handle if(if else) else

In a function where there are multiple returns, what is the best style to use?

This style, where there is no 'else' because it is frivolous and you think it is more concise?

f() {
    if (condition) {
        return value;
    }

    return other_value;
}

Or this, if you think that 'else' provides nice syntax sugar to make it more readable?

f() {
    if (condition) {
        return value;
    } else {
        return other_value;
    }
}

Feel free to share whatever other reasons you may have for preferring one over another.

EDIT:

The above samples are hypothetical, if they were this simple, as Yannis Rizos noted below, the ternary operator ( condition ? value : other ) would be acceptable. However I'm also interested in cases like this:

f() {
    if (condition1) {
        return value;
    } else if (condition2) {
        return value2;
    }
    ...
    else {
        return other_value;
    }
}
share|improve this question

marked as duplicate by Yannis Rizos, ChrisF Dec 28 '11 at 19:38

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

1  
You don't need else if the previous if is used to return. Mozilla coding guidelines require you to omit those elses. Otherwise, you may use a variabl, say, ret which you set accordingly in the If-Else ladder, and finally return ret in the end. I like to use switch-case particularly with this. –  yati sagade Dec 27 '11 at 8:13
    
@yati exactly why I asked the question, thank you for that lead. –  Tom Dignan Dec 27 '11 at 9:23
add comment

5 Answers

up vote 7 down vote accepted

I prefer this one:

return (condition) ? value : other_value;

but it's just a matter of personal preference. On any team I've worked I was happy to comply with whatever the code convention document preferred.

"Readability" is too vague a term to apply in such a specific and small issue and there's no technical reason to prefer one style over the others. Whichever one you feel is more readable, what matters most is to be consistent.

This StackOverflow question on Improving Code Readability gives some great points on other, more important, aspects of readability you should be concentrating on instead.


I got that the question was a hypothetical, but my answer still stands. It really doesn't matter.

On the expanded example, I prefer:

f() {
    if (condition1) {
        return value;
    } else if (condition2) {
        return value2;
    }
    ...

    return other_value;
}

But for a very specific reason: I organize the blocks in such a way that the last one, the return other_value; is the one that's more likely to happen, so my code hints to business logic - and I'm not pretending that's smart. Still a strictly personal preference, the only reason behind it is my personal process of organizing code, one that I'd happily avoid if it goes against the team's conventions.

In any case, if you have more than a couple consecutive else if blocks, rewriting into a switch statement would probably make more sense, readability-wise.

share|improve this answer
    
I also prefer to use the ternary operator when there is only one condition and the logic is simple, but what about more complex cases with multiple conditions? –  Tom Dignan Dec 27 '11 at 7:50
    
Forget about the example, and read the rest of the answer, it doesn't matter (unless of course you are polling people for opinions, in which case the question is off topic). And it's not the ternary operator, but a ternary operator. There are quite a few others. (Someone recently corrected me on this, just passing it along) –  Yannis Rizos Dec 27 '11 at 7:53
    
The question is of course subjective. The FAQ states that the guidelines allow for subjective questions when they are constructive and meet the six criteria. I believe my question qualifies. –  Tom Dignan Dec 27 '11 at 9:22
    
@ThomasDignan Of course it qualifies (otherwise I would have voted to close). My comment was on further pursuing the question, as if it was objective (which I didn't actually think you were). There's no correct answer, Mozilla coding guidelines may be a great place to start, but for every convention document advocating one way you'll find another advocating, well, another. See the question I've linked as a possible duplicate, there are some great answers there... (well I did vote to close, but as a duplicate) –  Yannis Rizos Dec 27 '11 at 9:36
add comment

Of course, as others have pointed out it is mostly a matter of personal preference. The key word here being mostly. There are, however, some objective guidelines that can be used, and personally, I like forming my personal preferences based on objective guidelines, regardless of how insignificant these may be.

So, the way I do it is as follows:

if( condition )
    return;
do-other-stuff;

And I always code multiple-if-else statements as follows:

if( a ) return x;
if( b ) return y;
if( c ) return z;
return null; //whatever, I ran out of letters

And here is why:

Deep down at the assembly language level, the 'else' keyword corresponds to some code. An if-then-else statement in assembly language looks like this:

    test condition
    if false, jump to else
    perform the if-part
    jump to end
else:
    perform the else-part
end:

The 'jump to end' instruction can be thought of as the code that the 'else' part corresponds to. Now, consider how the above code fragment changes when the if-part is a return statement:

    test condition
    if false, jump to else
    do-some-stuff-and-return
    jump to end
else:
    perform the else-part
end:

Clearly, the 'jump to end' instruction will never be executed. I would be willing to bet that when a compiler analyzes your code to find unreachable code, it does figure out that the 'else' keyword is unreachable, and it factors it out, but it refrains from issuing a warning because this style is used by many out there, and it does not want to upset them. But I use this as an objective justification, no matter how insignificant it is, for never using the 'else' keyword when the if-part returns.

share|improve this answer
    
Extremely insignificant, if true (too fuzzy to judge, and I haven't written a compiler in a while). But since I prefer never using else if if returns, good enough for me :) –  Yannis Rizos Dec 27 '11 at 8:37
    
In the last example I usually code it as as if (predicate) return functor(parameter);, giving the predicate and functor meaningful names. –  Alexander Galkin Dec 27 '11 at 9:37
    
@S.Lott you mean, the way I just rearranged it? –  Mike Nakis Dec 27 '11 at 12:11
    
@S.Lott OK, how about now? –  Mike Nakis Dec 27 '11 at 12:19
1  
@MikeNakis, Most of the time you should write the code that's as easy as possible to READ given an acceptable amount of time/effort required to write it. And when you do need to optimize, start by choosing smart algorithms and data structures. Not restructuring if-else branches. To quote Knuth: "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil". That said, if this method improves readability, use it. –  Brian McFarland Dec 27 '11 at 19:37
show 2 more comments

I like what Uncle Bob wrote in Clean Code. It was something along the lines that when you write clean and readable code, your functions should be small enough (and obviously fit on a single page) that you shouldn't need these type of guidelines or style specifications.

Instead your #1 objective is to improve readability as much as you can. As long as its readable and clear, anything else is just a personal preference. I'm assuming the example you supplied was just a hypothetical example but if it was actual code, then Yannis definitely gave you a more readable alternative.

share|improve this answer
    
Right, it was hypothetical. I would use the ternary operator if it was this simple. –  Tom Dignan Dec 27 '11 at 7:50
add comment

My personal preference is to include the unnecessary else. I find that I can grok the control flow at a glance when it's there, versus having to remember to hunt for returns that branch the flow implicitly. A couple times I've come back to cold code and added something after an "if" and been confused about why it wasn't running, only to realize I hadn't seen the return that bailed the function before getting to my new stuff.

share|improve this answer
    
This is why this is a problematic sort of question - there is no "best" - almost any choice is flawed (and just plain wrong in the eyes of some). I much prefer a single return point (there are reasons - not least of which is the case you describe) but that's at least in part a matter of my having in excess of 25 years commercial experience - I'm a dinosaur (-: –  Murph Dec 27 '11 at 14:52
    
@Murph - Well, the simplest solution to that problem is for everyone to admit that my way is best. I find that's the best solution to most any problem. :-) –  Dan Ray Dec 27 '11 at 15:05
    
the advantage of being a dinosaur, if you're technical lead (and I am) you get to exert a significant amount of influence over the coding standards (which should be agreed by concensus and which it should be possible to evolve - which it has to be possible to evolve given the emergence of things like var and dynamic in C#) –  Murph Dec 27 '11 at 15:38
add comment

The "single var pattern" often found in Javascript suggests single points of return by insisting that all variables are declared at the top of the function body to prevent "hoisting". Thus, recording the evaluations instead of returning can prevent the need for an ELSE (which I have a strange aversion to...):

 function doThis(required){
    var result = false;
    if(required){
        //base implementation
        result = doThat();
    }
    return result;
 }

If you have sequential or optional conditions and can't break out to another function, then you can do this:

 function doThis(required, opt){
    var result = false,
        oRequirement = 'foo';
    if(required){
        //base implementation
        result = doThat();
        if(result !== false && opt !== undefined && opt === oRequirement){
                //optional implementations
                result = doOptional();
            } else {
                //undo previous truthiness
                result = false;
            }
        }
    }
    return result;
 }

You could delegate consumption of variables externally, making this function effectively a "guard clause":

 function vA(a){
     return true;
 }

 function vB(c){
     return true;
 }

 function vB(c){
     return true;
 }

 function doThis(required, opt){
    var result = false;
    if(vA(required)){
        //base implementation
        result = doThat(required);
        if(vB(result) && vC(opt)){
            //optional
            result = doOptional(opt);
        }else{
            result = false;    
        }
    }
    return result;
 }

You could layer your evaluations:

 function doThis(required, opt){
    var result = false;
    if(if(vA(required)){
      //base
      result = doThat(required);
    }

    if(if(vB(result) && vC(opt)){
      //optional
      result = doOptional(opt);
    }
    return result;
 }

Ternaries can reduce size, and are very pretty, but they can be a little harder to debug, and a bit inconvenient if you need much in the way of nested evaluation steps:

function doThis(a, b, c){
    var o = false;
    if(valid(a)){ //assuming some sort of validation is required
        if(b !== undefined && valid(b)){
            o = c ? //an optional step
                doMore(a) ?
                    new Foo(a, b) :
                    false :
                new Bar(b, a) ;
        }
    }
    return o;
}

Generally speaking, I need to nest IF statements if optional arguments are present. When the outcome is the result of a process within the body, then it suggests delegating or chaining into a separate method.

Having said that, I can't help but feel that there is no "right" way (although there are certainly wrong ones...), and it's this fact that makes coding a creative profession capable of a broad range of expression. I mean, who doesn't love a bit of refactoring?

share|improve this answer
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.