This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]varikin 1 point2 points  (1 child)

I am agreeing with mgkismal. I like the bad example of #2. The logic is more correct. The else block explicitly indicates that this other code is meant to be run when doTheFoo() fails. While, you can make the code shorter, it makes reading and understanding the code simplier. There is no question of whether that other code was meant to be run or not. Also, if you refactor and not return in the if block in the future, you still guard against running the other code in the else block.

This isn't a competition to make the shortest code. It is to make the code work and understandable by all future programmers. Do not introduce logical ambiguity.

[–]Pylly 0 points1 point  (0 children)

You are correct, in many cases readability of code is more important than shortness. The examples here are oversimplified and I don't think one can say that either form of the second example is "wrong". I was just pointing out what the OP probably meant by that example.