views:

142

answers:

6

If I have something like a loop or a set of if/else statements, and I want to return a value from within the nest (see below), is the best way of doing this to assign the value to a field or property and return that?

See below:

bool b;

public bool ifelse(int i)
{
if(i == 5)
{
b = true;
}

else
{
b = false;
}
return b;
}
+7  A: 

what about

return i == 5;
gcores
I think that wouldn't have illustrated the "programming style" question he was trying to raise, but +1 for the most optimised answer.
DoctaJonez
the most optimized answer would have been "return true;" because the OP used the classic "if(i = 5)" vs. "if(i == 5)" (fixed)
BCS
I did notice that :) I figured because the language wasn't specified I wouldn't get hung up on it!
DaEagle
That's why I didn't comment, I just fixed it.
BCS
+6  A: 

There are multiple views on this. I think most people (including me) tend to prefer to return as soon as you have an answer and there is no more work to do. Some people will argue that you should only ever return at the last statement of a method. However, it can actually make things more complicated in some situations.

Going by what I've suggested, your example would be shorter and simpler:

public bool ifelse(int i)
{
if(i == 5)
{
return true
}
return false
}
DaEagle
+1 for answering the general question.
Greg D
A: 

I would say if you should generally only return from a method in two places - near the beginning (as in guard conditions) and near the end; If the method has any length to it, you should use a temporary variable as you mentioned, otherwise people reading the code may have a harder time following it.

Chris Shaffer
+5  A: 

If b is only used to calculate the return value for your method then you should make it local variable (defined within the method).

public bool ifelse(int i)
{
  bool b;
  /*
  Some code to calculate b
  */
  return b;
}

As others have suggested, If your method is simple I would avoid using a temporary variable altogether and return the result as soon as it is known. A general rule would be to use whichever method makes the code easiest to read.

foxy
+1 here. Variables should have the smallest scope necessary.
Jimmy
A: 

Yes, that is good style.

The alternative (which would be bad) would be to do this:


public bool ifelse(int i) 
{ 
    if(i == 5) 
    { 
        return true; 
    }
    else 
    { 
        return false; 
    }
}

The reason multiple return points are regarded as bad style is that especially for larger methods it can be difficult to keep track of the program flow within a method because it could exit at any point. This can be a nightmare to debug. If you have a return variable that you assign to however, you can watch that variable and know exactly when it will be returned (from a single place).

This isn't always the case, as with every stylistic point in programming there are good sides to it and bad sides.

DoctaJonez
Having long methods is regarded as bad style anyway. Usually shouldn't have methods more than about one page long. Any longer and they should be refactored to make them shorter. There are times that's not possible, but that's almost never.
DaEagle
Agreed, but this is a very short method. Where would you draw the line? A 3rd, 4th or 5th return point? Maybe even a 16th? It is a subjective thing (style always is) and it also depends upon circumstance. I think 2 return points are ok in the right situation, but more than that gets unmaintainable
DoctaJonez
Any numeric limit on returns above 1 will lead to nasty early-exit code "if (this || that || theother) return false;", instead of having three separate short early-exit lines for the three things that can cause early exit. I'd much rather maintain the latter than the former.
Steve Jessop
Although multiple returns do lead to occasional debugging hassle. Such as having to set multiple breakpoints, one on each return. Or having to rename the whole function and wrap it in a new version of the old name, in order to log the return value. Nothing you can't deal with as far as I've seen.
Steve Jessop
A: 

As pointed out, having more than one return statement has the downside of finding them gets hard. OTOH in some cases the added logic needed to escape to that return statement is worse that the problem the style is solving.

The major problem I'm aware of re multiple returns is that you can quickly forget to do some cleanup processing or the like at a new return point. IMHO this is just as much a problem with the single return form because the escape path has to remember to include that code and no other code. One solution to this, available in some languages like c#, is the finally block or it's neater form the scope statement as demonstrated here. (OK I'll get of my soap box now)

BCS