2

For example I can do:

public void onSomethingClick() {
   if (text.isEmpty()) {
      showMessage("The text is empty, put some text in the input");
   } else {
      //Do the things that you do if the text isn't empty here...
   }
}

Or I can do this:

public void onSomethingClick() {
   if (text.isEmpty()) {
      showMessage("The text is empty, put some text in the input");
      return;
   }
   //Do the things that you do if the text isn't empty here...
}

Sometimes it is more legible do it the first way, instead of keeping nesting if/else statements. Is there some convention about this? Any pitfalls to watch out for?

Jan Doggen
  • 8,799
  • 13
  • 70
  • 144
4gus71n
  • 3,717
  • 3
  • 39
  • 66
  • your code does not compiles – RamonBoza Nov 19 '13 at 10:21
  • Is just an example... I can do that in pseudocode, my point is if it's right use that coding conventions or not. – 4gus71n Nov 19 '13 at 10:24
  • It depends on your logic.If you want to return immediately after showMessage(), you can do return there itself.If you have other code in the function after if else loop,then do return atlast. – Renjith Nov 19 '13 at 10:24
  • Also, I must add that [eclipse kelper](http://download.eclipse.org/eclipse/downloads/drops4/R-4.3-201306052000/news/eclipse-news-part2.html) has added a feature called 'Convert to if-!-return' that allows the user to automatic generate this kind of guard sentences – 4gus71n Dec 02 '13 at 00:14

5 Answers5

2

The second form uses a guard condition which is pretty common. Most programmers should be able to understand the logic and follow the flow of the code. I would use the second form since the extra else is just syntactic sugar.

See this writeup on Wards Wiki.

Kevin Bowersox
  • 93,289
  • 19
  • 159
  • 189
2

There's no common convention for this - either way is fully acceptable. However it is good practice to be consistent with which way you use throughout your whole code file / project / solution. If I had to choose one or the other, it would be the option with return. Some popular code-reviewing tools such as ReSharper openly recommend this option.

Also, one variant you should definitely avoid is combining the two:

public void onSomethingClick() {
    if (text.isEmpty()) {
    showMessage("The text is empty, put some text in the input");
    return;
    } else {
  //Do the things that you do if the text isn't empty here...
    }
}

as the else here is clearly redundant, since you would achieve the same result without including it.

Alex Walker
  • 2,337
  • 1
  • 17
  • 32
  • The else block will *not* be redundant when the `if` condition is false. – Manish Nov 19 '13 at 10:27
  • 1
    It will be redundant - in the sense that you would achieve exactly the same effect by taking the `else` and the enclosing braces out of the code. – Alex Walker Nov 19 '13 at 10:28
  • Got it. Makes sense now. – Manish Nov 19 '13 at 10:29
  • +1 I agree that consistency is good - but that doesn't necessarily mean use only one or the other. If the `if` clause is a short guard condition or possible short-circuit, then to me it makes sense to `return` in those conditions. But if it's one of two plausible paths for the method itself, both of equal length/weight, `if/else` seems to make more sense. It's a judgement call, ultimately. – Andrzej Doyle Nov 19 '13 at 10:52
2

Both approaches are valid, depending on the circumstances. All else being equal, I would favour the first approach because it is easy to see at a glance that there are two branches to the code; having the two possible paths for the code being at the same level of indentation emphasises the "peer" nature of the two branches.

On the other hand the second approach is useful when one branch is significantly longer; if the clause is e.g. simply to flag missing data it is acceptable to get this out of the way before reaching the main function body, avoiding extra indentation and cognitive load.

In your specific example it looks like this second view of the code might be a better match.

This question may be relevant: Should a function have only one return statement?

Community
  • 1
  • 1
Rich Smith
  • 1,675
  • 12
  • 24
1

My personal rule of thumb is something like this:

void doStuff(param) {
    if (!isValid(param)) {
        //handle error
        return;
    }
    // Do complex logic here
}

Basically, if you have some short code that can execute but isn't the main feature of the method, then doing an if-handle-return without an else is the way to go imo.

If the two pieces of code are of equal importance (for instance, do operation A for odd numbers and operation B for even numbers), then I use if-else.

BambooleanLogic
  • 7,530
  • 3
  • 30
  • 56
0

When you are testing a precondition at the start of a function, as in your example, it makes perfect sense to return from inside the if branch, as there is nothing else your function can do.

It is a little more dangerous to return from a nested statement or from the middle of a function, as this may inadvertently skip some required processing or cleanup.

Nicola Musatti
  • 17,834
  • 2
  • 46
  • 55