Programming preference - use else ifs with multipl

问题:

The code:

public String getTemperatureMessage(double temp)
{
    if(temp < 32)
        return "Freezing";
    else if(temp < 60)
        return "Brr";
    else if(temp < 80)
        return "Comfortable";
    else
        return "Too hot";
}

Regarding the code snippet above, the else ifs are technically superfluous and don't change the behavior at all. However, I tend to like to put them in there to emphasize that the conditions are exclusive. What are your thoughts? Unnecessary or more clear?

回答1:

Some would say that multiples return would be the problem here. But it's not really my point.

For my point of view, the if/else if is really important, because even if in your case you return some value, removing the elses would mean that you wouldn't put them anyway, and that would mean a totally different thing if the returns were not here.

Plus, imagine someday someone want to edit your code, and clean it up for a single return, this person could misunderstand your code and do a grave mistake like this :

public String getTemperatureMessage(double temp){
    String message;
    if(temp < 32)
        message = "Freezing";
    if(temp < 60)
        message = "Brr";
    if(temp < 80)
        message = "Comfortable";
    else 
        message = "Too hot";
    return message;
}

To clarify my point of view, keep the elses, it keep your code clear.

回答2:

The only feasible alternative in this particular case is to grab the conditional operator ?:.

public String getTemperatureMessage(double temp) {
    return temp < 32 ? "Freezing"
         : temp < 60 ? "Brr"
         : temp < 80 ? "Comfortable"
         : "Too hot";
}

Left behind the question how that's readable to starters.

References

• JLS 15.25 Conditional Operator ?:
  • The operator is right-associative, thus such nesting of the operator will work "as expected"

Related questions

• To ternary or not to ternary?

回答3:

It depends on a lot of things like how complex your code is. With a simple example like this, I'd put the returns on the same line as the ifs, and not use elses. The structure and behaviour is clear:

public String getTemperatureMessage(double temp)
{
    if(temp < 32) return "Freezing";
    if(temp < 60) return "Brr";
    if(temp < 80) return "Comfortable";
    return "Too hot";
}

When I have more complex code I find it useful to not break out from the nesting with returns or continue/break, but to assign to state or result variables. I will then also include {} even if the block is a single statement, mostly to keep the consistency in how the structure is represented in code, but also to slightly reduce the risk that later edits will forget to change a statement to a block.

If this example was more complex, I'd probably code it like this:

public String getTemperatureMessage(double temp) {
    String result;
    if(temp < 32) {
        result = "Freezing";
    } else {
        if(temp < 60) {
            result = "Brr";
        } else {
            if(temp < 80) {
                result = "Comfortable";
            } else {
                result = "Too hot";
            }
        }
    }
    return result;
}

回答4:

If a function has multiple "successful" return values, I'll use if/else to select among them. If a function has a normal return value, but one or more ways that may abnormally exit early, I generally won't use an "else" for the normal path. For example, I think it's far more "natural" to say:

int do_something(int arg1)
{
  if (arg1 > MAX_ARG1_VALUE)
    return ARG1_ERROR;
  ... main guts of code here
  return 0;
}

than to say:

int do_something(int arg1)
{
  if (arg1 > MAX_ARG1_VALUE)
    return ARG1_ERROR;
  else
  {
    ... main guts of code here
    return 0;
  }
}

or

int do_something(int arg1)
{
  if (arg1 <= MAX_ARG1_VALUE)
  {
    ... main guts of code here
    return 0;
  }
  else
    return ARG1_ERROR;

This distinction becomes especially significant if there are multiple things that can "go wrong", e.g.

int do_something(int arg1)
{
  if (arg1 > MAX_ARG1_VALUE)
    return ARG1_ERROR;
  ... some code goes here
  if (something_went_wrong1)
    return SOMETHING1_ERROR;
  ... more code goes here
  if (something_went_wrong2)
    return SOMETHING2_ERROR;
  ... more code goes here
  if (something_went_wrong3)
    return SOMETHING3_ERROR;
  return 0;
}

Nested 'if/else' statements in such cases can get ugly. The most important caveat with this approach is that any cleanup code for early exits must be given explicitly, or else a wrapper function must be used to ensure cleanup.

回答5:

For simple 1-liners I tend to leave out the else but if there's more complex if blocks I tend to prefer the else to make it clear that the conditions are mutually exclusive.

回答6:

public String getTemperatureMessage(double temp)
{
    String retval = null;
    if(temp < 32)
        retval = "Freezing";
    else if(temp < 60)
        retval = "Brr";
    else if(temp < 80)
        retval = "Comfortable";
    else
        retval = "Too hot";
    return retval;
}

回答7:

For a simple if statement without too many lines of code having multiple returns is no problem. However, nothing infuriates me quite so much as:

function doTemperatureCalculations(double temperature) {
  if (temperature < 20) {
    /* 
      Gazillion lines of code here .....
     */
    return "Very cold!";
  } else if (temperature < 40) {
    /*
      Another gazillion loc .....
     */
    return "Summer in the North Pole.";
  } else {
    /*
      Multiple returns embedded throughout ....
     */
  }
}

回答8:

In this case it is more clear. In the general case you may want to leave the elses off as they can contribute to more nesting and code complexity. For example:

if (error condition) {  
  do some stuff;
  return;
} else {
  do stuff;
  if (other error condition) {
     do some stuff1;
     return;
  } else {
     do some other stuff;
     return

  }
}

The code below keeps the level of nesting down which reduces code complexity:

if (error condition) {
  do some stuff;
  return;
}
do stuff;
if (other error condition) {
  do some stuff1;
  return;
}
do some other stuff;
return;

In your example it is easy either way. But in many cases you would be better off using a lookup table for this sort of thing and reading the values from a file/database. For efficiency in C often this would be coded as an array of structs.

The else does add some clarity in that it makes it clear that the cases are mutually exclusive. However the idiom of returning like you do is obvious to many programmers, so either way the majority will know what you meant.

I can think of an advantage of the elses. If you want to add a new last case, without the elses you might forget to add an if to the currently "Too Hot Condition" if say you wanted to add "Dying" at 120 or something. while with the elses you know that you need the final else to be in front of "Dying" so you will be more likely to think about putting the else if in front of "Too Hot". Also if you just put an else on "Dying" you will get a compile error which forces you to think.

回答9:

Personally, I think the elses are unnecessary. Since this question is tagged as [language-agnostic], I'm going to provide a couple of examples of how I would write it:

def temperature_message(temp)
  return 'Freezing'    if temp < 32
  return 'Brr'         if temp < 60
  return 'Comfortable' if temp < 80
  'Too hot'
end

This is typical guard clause style, which both I personally and the Ruby community in general use quite often.

def temperature_message(temp)
  case
  when temp < 32
    'Freezing'
  when temp < 60
    'Brr'
  when temp < 80
    'Comfortable'
  else
    'Too hot'
  end
end

This is a typical switch as you would find it in some less powerful languages. This is probably one that I would not use, I would refactor it like this:

def temperature_message(temp)
  case temp
  when (-1.0/0.0)...32
    'Freezing'
  when 32...60
    'Brr'
  when 60...80
    'Comfortable'
  else
    'Too hot'
  end
end

Although I must admit I still find the first one easiest to read.

Since this is basically a mapping table, I would try to format it as such, so that everybody who reads the code, immediately sees the "table-ness":

def temperature_message(temp)
  case temp
  when (-1.0/0.0)...32 then 'Freezing'
  when         32...60 then 'Brr'
  when         60...80 then 'Comfortable'
                      else 'Too hot'
  end
end

This also applies to your original Java implementation:

public String getTemperatureMessage(double temp) {
    if(temp < 32) return "Freezing";
    if(temp < 60) return "Brr";
    if(temp < 80) return "Comfortable";
    else          return "Too hot";
}

Of course, since it is basically a mapping table, you might just as well implement it as a map:

def temperature_message(temp)
  {
    (-1.0/0.0)...32       => 'Freezing',
            32...60       => 'Brr',
            60...80       => 'Comfortable',
            80..(1.0/0.0) => 'Too hot'
  }.detect {|range, _| range.include?(temp) }.last
end

回答10:

The redundant elses make me cringe. The extra syntax and indentation makes it harder for me to read. I just got through removing a bunch of these from some code I inherited.

Most cases of redundant code are mistakes, so by implication a redundant 'else' looks like a mistake to me even if you put it there on purpose. The impression I get is of code that was originally written without embedded returns, then someone rewrote it to have the embedded returns, but they were too lazy to remove the elses.

A single if/return is easy to understand; so are 4 in a row. It's "that case is done; let's move on". A long chain of if/elses can be a pain to read; you need to read all the way to the bottom before you find out what happens. The saving grace is it allows a single return to be used -- a feature that is overrated IMO but I'll admit it does provide some value. However a long chain of if/elses combined with returns mixed in amongst the elses is the worst of both worlds -- all the disadvantages of multiple returns, and made to look like one large construct you have to get in your head all at once. Ugh.

From a theoretical perspective consider this: The zone between the return and the else is essentially unreachable code. Sure, it only contains whitespace, but the zone shouldn't even be there at all.

Finally, an example of if/return/else taken to its redundant conclusion. I've seen a few of these lately. Why in the world is there an else block? The code in the else block executes under the same conditions as the code directly after it:

...
if (temp < 35)
{
  foo.status = TOO_COLD;
  return;
}
else
{
  foo.status = TEMP_OKAY;
}
launch_the_rocket(now);
return;

回答11:

I agree that the elses makes it more clear. The final else especially helps to make a visual distinction between the case where every branch has a return, and the cases where only some branches have a return (which could be a smell).

回答12:

There's no point. You're adding needless semantic and other overheads for absolutely zero benefit. When you return, you return and control is over. Pretending anything else is superfluous and just makes you look like you don't know what the return statement does.

回答13:

Who uses IF/ELSE IF... when you can use a SWITCH statement?

My preference is for the single RETURN statement - multiple return statements can make debugging a pain...

回答14:

I don't think I'd write in this way in the first place, but going along with the premise, I'm guessing that any compiler would crank out the same code whichever method you chose to write, so there is no technical reason that I can think would favour one over the other.

The argument is therefore Single or Compound statement.

I think the rule of "least surprise" should be used, which in this case would call FOR the superfluous else statements to be included.

PS. I would always send a temperature in degrees centigrade, oops, just broke your function!

回答15:

it's good. without "else", this line

if(temp < 80)
    return "Comfortable";

would be unconvincing. with "else", it's clear that there are other preconditions.

回答16:

I prefer case statements myself but that would make this a language specific topic and not language-agnostic topic.

    Dim Temp As Integer
    Dim Message As String

    Select Case Temp
        Case Is < 32
            Message = "Freezing"
        Case Is < 60
            Message = "Just Right"
        Case Is < 80
            Message = "Too Hot"
        Case Else
            Message = "What was I doing?"
    End Select

I find these alot easier to read then if..else statements.