Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have coded like this many times, and I've never encountered an issue, but the compiler always warns when it expects a return and there is none.

For instance, look at this:

-(NSString *)outputStringForInteger:(NSInteger)int
{
    if (int == 0)
    {
        return @"Number is Zero";
    }
    else
    {
        return @"Number is not Zero";
    }
    //no "failsafe" or other explicit return
}

If the function will never get to the last line, ever, is it important to still have a failsafe option, or do you guys just deal with compiler warnings?

share|improve this question
12  
How about skipping the else clause and just returning? –  badp Nov 1 '10 at 18:43
    
That's a good suggestion, thanks! –  Justin Amberson Nov 1 '10 at 18:47
    
Your opening and closing braces don't match up. –  John Nov 1 '10 at 19:42
2  
how is it a 'failsafe' if it can't happen? what failure are you protecting yourself from? –  GSto Nov 1 '10 at 19:51
1  
@TheLQ: guessing from the syntax I'd say that's Objective-C. –  Baelnorn Nov 1 '10 at 20:33
show 2 more comments

7 Answers

Get a better compiler. The ones I use complain that you have code which will never execute if you put the "failsafe" return in there. That's a much better warning that the false one you're apparently seeing.

share|improve this answer
    
Yeah, that's what I thought when I first saw that code sample. What kind of compiler are you using that can't perform branch analysis properly? –  Mason Wheeler Nov 1 '10 at 20:53
6  
Indeed, I have never seen a compiler which would warn about a non-problem like that, and if I did, I'd be terrified about how many more important things the compiler might also have implemented incorrectly. –  Carson63000 Nov 1 '10 at 22:08
    
This is the XCode compiler for Objective-C, in methods that specify a return type, when there is not a return before the last brace, it gives a warning that there is no return value. –  Justin Amberson Nov 2 '10 at 17:08
add comment

Not sure what language you're in so I'll use Java for my example.

String outputStringForInteger(int i) {
   String returnString = "Number is not Zero";

   if (i == 0)
      returnString = "Number is Zero";

   return returnString;
}

This will provide a "default" return value, as well as being more readable(IMO).

share|improve this answer
3  
Looks a lot less readable to me than just an if...return; return flow –  TheLQ Nov 1 '10 at 20:26
3  
I consider this paradigm very unreadable, because the first assignment to returnString assumes the number is not zero without every querying the number's value, meaning it is an incomplete line that you have to hold in your head until you finish with the if until it makes any sense. Much better to declare returnString and then assign the 'Number is not Zero" string in the else. –  Yishai Nov 1 '10 at 21:26
    
That's horrible. You can write it in just a line: return i == 0 ? "Number is zero" : "Number is not zero" - or at least I would do that in C#. Dunno if java has the conditional operator, but I suppose there's something equivalent. –  Matteo Mosca Nov 2 '10 at 16:20
    
That will work also; I just prefer to only use ternary for boolean and occasionally ints(where there's clearly only 2 possible returns). For highly maintained code, what if we need to add another condition? granted, in this case there are only 2 return possibilities. –  Michael K Nov 2 '10 at 16:30
add comment

In C:

String outputStringForInteger(int i) {
   return (i == 0) ? "Number is Zero" : "Number is not Zero";
}

If you insist that the ternary operator is an invention of the Devil, as some do:

String outputStringForInteger(int i) {
   String returnString;

   if (i == 0) {
      returnString = "Number is Zero";
   } else {
      returnString = "Number is not Zero";
   }
   return returnString;
}
share|improve this answer
    
Did C add the String type? –  Michael K Nov 1 '10 at 19:51
1  
@Michael: I assumed the existence of a String type, either system- or programmer-supplied. Makes no difference. If you want to be picky, assume "typedef char *String;" appears somewhere above the fragment. –  John R. Strohm Nov 1 '10 at 21:36
    
() is unneeded. –  user1249 Nov 1 '10 at 21:54
    
@Thorbjørn, I have occasionally seen notes about weirdness in operator precedence in C, and I have gotten in the habit of parenthesizing things a bit more than strictly necessary. Doing it this way, knowing I'll never have to look at it again, is a bit more important to me than achieving the strict minimum possible number of keystrokes. –  John R. Strohm Nov 2 '10 at 17:13
add comment

I prefer not to have multiple returns in a function.

share|improve this answer
1  
I would almost always agree. (There's always an exception somewhere...) –  Michael K Nov 1 '10 at 19:29
13  
why not? multiple exit points can make a function a lot easier to read at times. I'd rather see if(condition) return false; at the top of a long function that have to read through the whole thing to see that that was the result. –  GSto Nov 1 '10 at 19:53
2  
Yes of course there are exceptions but all things being equal I like one exit point. –  Gratzy Nov 1 '10 at 20:26
2  
Practicing 'fail fast' at the function level almost always leads to multiple returns (or throws) and can appear very clear. –  Steve Evers Nov 1 '10 at 23:39
1  
@GSto, a function long enough for multiple exits to make it more readable is definitely a code smell. –  John R. Strohm Dec 16 '10 at 4:19
show 3 more comments

In D, except in a few trivial cases where the compiler can statically prove that the end of the function is unreachable without a return statement being executed or an exception being thrown, you must put either a return statement, a throw statement or an assert(0) at the end of the function. In other languages I think this is usually good practice, though not necessarily in cases like your example, where a reader could trivially and statically tell that the end of the function is unreachable and a decent compiler should be able to prove this.

You shouldn't have a default return value that you believe is unreachable. If it ever gets used your code is by definition in a state that you never intended for it to be in. An assert(0) conveys your intentions both to the reader and the compiler much better.

share|improve this answer
add comment

As long as you have covered all the possible code paths, you're fine.

If your compiler is complaining, there's two possibilities:
1) You think you covered everything and didn't.
2) Your compiler is brain dead.

That said, there's most likely a way that you can restructure the code to placate the compiler and still cover the code paths. (e.g., drop the else in your example)

share|improve this answer
add comment

How about this (Java - I don't know the OP's language)?

String outputStringForInteger(int i)
{

   if (i == 0)
      return "Number is Zero";

   return "Number is not Zero";
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.