4

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?

6
  • 13
    How about skipping the else clause and just returning?
    – badp
    Commented Nov 1, 2010 at 18:43
  • Your opening and closing braces don't match up.
    – John
    Commented Nov 1, 2010 at 19:42
  • 2
    how is it a 'failsafe' if it can't happen? what failure are you protecting yourself from?
    – GSto
    Commented Nov 1, 2010 at 19:51
  • Is that C++ or something?
    – TheLQ
    Commented Nov 1, 2010 at 20:25
  • 1
    @TheLQ: guessing from the syntax I'd say that's Objective-C.
    – Baelnorn
    Commented Nov 1, 2010 at 20:33

7 Answers 7

22

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.

3
  • 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? Commented Nov 1, 2010 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. Commented Nov 1, 2010 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.
    – Daddy
    Commented Nov 2, 2010 at 17:08
6

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).

4
  • 3
    Looks a lot less readable to me than just an if...return; return flow
    – TheLQ
    Commented Nov 1, 2010 at 20:26
  • 4
    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
    Commented Nov 1, 2010 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. Commented Nov 2, 2010 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
    Commented Nov 2, 2010 at 16:30
4

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;
}
4
  • Did C add the String type?
    – Michael K
    Commented Nov 1, 2010 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. Commented Nov 1, 2010 at 21:36
  • () is unneeded.
    – user1249
    Commented Nov 1, 2010 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. Commented Nov 2, 2010 at 17:13
2

I prefer not to have multiple returns in a function.

8
  • 1
    I would almost always agree. (There's always an exception somewhere...)
    – Michael K
    Commented Nov 1, 2010 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
    Commented Nov 1, 2010 at 19:53
  • 2
    Yes of course there are exceptions but all things being equal I like one exit point.
    – Gratzy
    Commented Nov 1, 2010 at 20:26
  • 2
    Practicing 'fail fast' at the function level almost always leads to multiple returns (or throws) and can appear very clear. Commented Nov 1, 2010 at 23:39
  • 1
    @GSto, a function long enough for multiple exits to make it more readable is definitely a code smell. Commented Dec 16, 2010 at 4:19
1

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.

1

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)

1

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";
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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