Why is "still" rarely used after "if x then return"?

Question

This method:

boolean containsSmiley(String s) { if (s == null) { return false; } else { return s.contains(":)"); } } 

can be written equally:

 boolean containsSmiley(String s) { if (s == null) { return false; } return s.contains(":)"); } 

In my experience, the second form is considered more often, especially in more complex methods (where there can be several such exit points), and the same is true for the “throw” as well as the “return”. However, the first form may make the conditional structure of the code more explicit. Are there any reasons to prefer each other?

(Related: If a function has only one return statement? )

Answer 1

In my experience, it depends on the code. If I "protect" from something, I will do:

 if (inputVar.isBad()) { return; } doThings(); 

Clear. If this statement is false, I do not want the function to continue.

On the other hand, there are several functions with several options, in which case I would write it as follows:

 if (inputVar == thingOne) { doFirstThing(); } else if (inputVar == secondThing) { doSecondThing(); } else { doThirdThing(); } 

Even if it can be written as:

 if (inputVar == thingOne) { doFirstThing(); return; } if (inputVar == thingTwo) { doSecondThing(); return; } doThingThree(); return; 

In fact, it’s completely clear what the code does (it’s not necessary which bit of code is the shortest or least indented).

Answer 2

else in this case will be redundant, and also create an unnecessary additional indent for the main function code.

Answer 3

This is a template called a reservation clause . The idea is to keep all checks up to reduce nested conditions to increase readability.

From the link:

 double getPayAmount() { double result; if (_isDead) { result = deadAmount(); } else { if (_isSeparated) { result = separatedAmount(); } else { if (_isRetired) { result = retiredAmount(); } else { result = normalPayAmount(); } } } return result; } 

Using the Guard clause, you will see this result:

 double getPayAmount() { if (_isDead) return deadAmount(); if (_isSeparated) return separatedAmount(); if (_isRetired) return retiredAmount(); return normalPayAmount(); }; 

Answer 4

You will see it everywhere:

 if (condition) { return var; } // by nature, when execution reaches this point, condition can only be false, // therefore, the else is unnecessary return other_var; 

In most cases, adding an else clause is not only optional in this case, but many times, it is optimized by the compiler.

Think about how the computer thinks about this code (in terms of machine code simplified for pseudocode here for demo purposes):

 0x00: test [condition] 0x01: if result of test was not true, goto [0x04] 0x02: push [var] onto stack 0x03: goto [0x05] 0x04: push [other_var] onto stack 0x05: return from subroutine 

The code (again, this is pseudocode, not assembly) will act in exactly the same way for conditional if/then/else .

Many people believe that bad and / or confusing practice has several possible exit points for a function, since the programmer must think about every possible path through his code. Another practice is as follows:

 return (condition) ? var : other_var; 

This simplifies the code and does not create any new exit points.

Answer 5

I prefer to write it like this:

 boolean containsSmiley(String s) { return s != null && s.contains(":)"); } 

Answer 6

Like any "discussion" about coding styles, there is no right answer. I prefer to apply these considerations:

• Does the code work on hold in all situations. (The principle of least surprise)

• Can the next developer (me or someone else) understand what he is doing and why.

• How fragile the code is regarding change.

• It is simple, as it should be, and nothing more. That is, not above or under engineering.

As soon as I am happy that I have satisfied the above, the rest, as a rule, simply fall in line.

Answer 7

else is redundant. In addition, some IDEs (Eclipse) and analysis tools (possibly FindBugs) may flag this as a warning or error, so programmers can remove it in this case.

Answer 8

This is a religious argument, and at the end of the day it does not matter. I even argue that the first form is more readable in some cases. If you have large chunks of code in if-elseif-elseif-else , it’s easier, at first glance, to see what the default return is.

 if (s == null) { return false; } else if (s.Contains(":))")) { return true; } else if (s.Contains(":-(")) { return false; } return s.contains(":)"); 

Answer 9

The Ockham razor is a principle according to which "entities should not be multiplied beyond necessity."

Answer 10

The if checks / applies your contract / waiting for not getting null values. For this reason, I would prefer it to be separated from the rest of the function, since it has nothing to do with the actual logic of what you are trying to execute (although this case is very simple).

In most cases, I prefer the code to be as explicit as possible in my intent. If there is something that you can restructure about your function to make it more readable to others, do it. As a professional programmer, your goal should really be intended for those who should support your code after you (including you after 2 years ...). Everything you can do to help them is worth doing.

Answer 11

Because it is better. You know that you can also use '{' '}' to create multiple levels of nesting, but no one does it just for this.

Answer 12

Someone might already have noticed this, but I would recommend not using null values ​​at all where strings are expected. If you really want to verify that someone is missing null values, you can use statements (at dev time) or unit tests (expand):

 boolean containsSmiley(String s) { assert s != null : "Quit passing null values, you moron."; return s.contains(":)"); } 

I moved on to the general rule: Never. Ever. pass null values ​​unless external API calls explicitly request it. Second: if the external method can return null values, replace it with a reasonable non-zero value (for example, an empty string) or add a neat check. I am sick with repeated if (thing == null) checks.

But this is a bit offtopic. I like to put short terms in overhead and defensive sentences, removing elses if the program flow tells me that it will never get there.

Answer 13

The first form is simply less details - when you return a value, you automatically leave the scope of the function you are in and return to the caller, so any of the following code will be executed only if the IF statement does not evaluate to true and subsequently return something.

Answer 14

I would argue about readability. If you scan code screens trying to figure out what the code is doing, this is a visual hint to the developer.

... but it really isn’t necessary, because we all comment on our code so well, right? :)

Answer 15

In my opinion, the second makes more sense. It serves more as a default action, such as a switch. If it does not match any of the other exit points, do it. You really don't need anymore. I would say if the whole function will be only if elseif, then another will make sense there, because this is one conditional giant. If there are several conditional expressions and other functions in it, then the default return at the end will be used.

Answer 16

Although the presence of else is correct and there is nothing wrong with it in terms of logic and performance, I like to avoid the initial point of WTF when a function does not have a return statement outside the if / else scope.

Answer 17

As you can see, different people have different opinions regarding readability. Some people think that fewer lines of code make the code more readable. Others believe that the symmetry of the second form makes it more readable.

I believe that probably both views are correct ... for the people who hold them. And the consequence is that you cannot write code that everyone finds optimally readable. So, the best advice is to follow what your approved coding standard says (if it says something about it) and usually uses your common sense. (If you are burdened by some loud nitwit that insists that its path is "correct" ... just go with the flow.)

Answer 18

Because there is a warning in eclipse (off by default), if something else is used in this situation :;).

Answer 19

Well, some of the reasons are just an agreement, but there is one advantage in the form above ...

This is typical of encoding a return statement to make the last expression its default return value. This primarily helps during refactoring - other sentences, as a rule, are wrapped in other structures or may accidentally move into a tree.

Answer 20

I would prefer a single exit point across multiple service points. The end result can be changed (or decorated) at one exit point, and not at n exit points.

Answer 21

The second form, if simpler / shorter. This does not always mean clarity. I suggest you do what you find most clear. Personally, I would write.

 static boolean containsSmiley(String s) { return s != null && s.contains(":)"); } 

Answer 22

Because it is the same as writing one of the following, which gives evidence of the programmer’s intention:

 boolean containsSmiley(String s) { if (s == null) // The curly braces are not even necessary as the if contains only one instruction. return false; return s.contains(":)"); } 

Or even this:

 boolean constainsSMiley(String s) { return string.IsNullOrEmpty(s) ? false : s.Contains(":)"); } 

These two forms:

• More elegant;
• Easier to read;
• Leaner and swifter for a reading programmer.