views:

433

answers:

10

Given the following code snippets, is there any appreciable difference?

public boolean foo(int input) {
   if(input > 10) {
       doStuff();
       return true;
   }
   if(input == 0) {
       doOtherStuff();
       return true;
   }

   return false;
}

vs.

public boolean foo(int input) {
   if(input > 10) {
      doStuff();
      return true;
   } else if(input == 0) {
      doOtherStuff();
      return true;
   } else {
      return false;
   }
}

Or would the single exit principle be better here with this piece of code...

public boolean foo(int input) {
   boolean toBeReturned = false;
   if(input > 10) {
      doStuff();
      toBeReturned = true;
   } else if(input == 0) {
      doOtherStuff();
      toBeReturned = true;
   }

   return toBeReturned;
}

Is there any perceptible performance difference? Do you feel one is more or less maintainable/readable than the others?

+4  A: 

Use whatever form best describes your intent.

Do not follow the single exit principle if things are this simple, though--it just makes it more confusing.

Michael Haren
A: 

Semantically — no. Performance-wise this depends on compiler, i.e. whether it can spot that both conditions cannot be true at once. I'd bet standard Sun compiler can. Whether to use single exit principle depends on tastes. I personally hate it.

doublep
A: 

Between the first and second snippets, there's really no difference. However the third snippet is quite inefficient. Because you wait to return control of the program to the caller until the last line of code in the method, you waste processing power/memory whereas the first two code snippets return control as soon as it determines one of the conditions to be true.

KSwift87
You're almost certainly incorrect - any modern compiler will optimize each of these to the same code. And regardless, 'returning control to the program' takes no processing power or memory
KevinDTimm
Perhaps you misunderstood. Where I meant the waste of processing power occurs is if the first condition validates to true. In his first two examples the flow of control would be returned to the calling method instead of continuing on with pointless validation. So correct me if I'm wrong, but to say that executing additional *unnecessary* code "takes no processing power" isn't true, no?
KSwift87
Nope. Following your sentence semantically, 'the third snippet is quite inefficient. Because you wait to return ... until the last line of code ... you waste processing power'. You critique only the third snippet as you have dismissed 1 and 2 already. My assertion stands.
KevinDTimm
After rereading my answer, you're right thanks to my semantics. I did not explain thoroughly why I originally thought snippet 3 could waste processing time/power. I also realize after rereading the OP that the point I was going to make about snippet 3 would be moot thanks to the "else" part of the code.Anyway I at least took notice of the "toBeReturned" variable upon rereading the OP. That variable (besides helping me somewhat save-face) creates more overhead (takes more memory), making it inefficient. So, to the OP, don't code blocks like snippet 3 if you can avoid it. :)
KSwift87
+10  A: 

With the second example you state very clearly that both conditions are mutually exclusive.
With the first one, it is not so clear, and in the (unlikely) event that an assignment to input is added between both ifs, the logic would change.
Suppose someone in the future adds input = 0 before the second if.
Of course this is unlikely to happen, but if we are talking about maintainability here, if-else says clearly that there are mutually exclusive conditions, while a bunch of ifs don't, and they are not so dependent between each other as are if-else blocks.

edit:Now that I see, in this particular example, the return clause forces the mutual exclusivity, but again, we're talking about maintainability and readability.

Anyway, about performance, if this is coded in Java you shouldn't care for performance of a couple of if blocks, if it were embedded C in a really slow hardware, maybe, but certainly not with java.

Petruza
A: 

As far as I can see, there is no difference between what the three snippets will do/return, but I would personally go with the second one, as it's good practice. That way you avoid several if-clauses to run when you only want one. But then again in this case you are returning in each if clause (at least in the two first snippets), so it wouldn't matter.

When it comes to performance I'm not really sure which is fastest, but I am pretty sure the difference is minuscule.

Harpyon
A: 

Version #1 and #2 may be faster than #3, but I suppose the performance difference is minimal. I would rather focus on readability.

Personally, I would never use version #2. Between #1 and #3, I would choose the one that yields the most readable code for the case in question. I don't like many exit points in my methods, because it makes the code hard to analyze. However, there are cases where the flow becomes clearer when we exit immediately for some special cases, and continue with the main cases.

Eyal Schneider
A: 

Think of this case when the two examples won't be similar:

    public boolean foo(int input) {
        if (input > 10) {
            // doStuff();
            return true;
        }
        System.out.println("do some other intermediary stuff");
        if (input == 0) {
            // doOtherStuff();
            return true;
        }

        return false;
    }

vs.

    public boolean foo(int input) {
        if (input > 10) {
            // doStuff();
            return true;
        } 
        //System.out.println("doing some intermediary stuff... doesn't work");
        else if (input == 0) {
            // doOtherStuff();
            return true;
        } else {
            return false;
        }
        return false;
    }

The first approach is probably more flexible, but both formulas have their use in different circumstances.

Regarding performance, I think the differences are to small to be taken in consideration, for any regular java application, coded by sane programmers :).

Andrei Ciobanu
A: 

In your case the second if would only get called if the first if failed so it's less important here but if your first if did something and didn't return, the second if (which would then always be false) would still be tested unless it was in an else-if.

In other words, there are cases where the difference between if-else-if and if-if matters, but this isn't one of them.

Example: Try this and then try it after removing the else. You get two different outputs:

int someNumber = 1;
if(someNumber < 5)
{
    someNumber += 5;
    Console.WriteLine("First call.");
}
else if(someNumber >= 5)
{
    Console.WriteLine("Second call.");
}
Nick Gotch
+4  A: 
  • In the first:

    somebody eventually, by some strange reason and when you're not looking will add some add statement that will make this method fail under certain strange conditions, everybody ( or worst, one single person ) will spend 4 hrs. watching the source code and debugging the application to finally found there was something in the middle.

  • The second is definitely better, not only it prevents this scenario, but also helps to clearly state , it this or this other no more.

    If all the code we write within an if where 10 lines long at most, this wouldn't matter really, but unfortunately that's not the case, there exists other programmers which by some reason think that a if body should be > 200 lines long... anyway.

    • I don't like the third, it forces me to look for the return variable, and it's easier to find the return keyword

About speed performance, they are ( almost ) identical. Don't worry about that.

OscarRyz
A: 

In your last example, don't do this:

public boolean foo(int input) {
   boolean toBeReturned = false;
   if(input > 10) {
      doStuff();
      toBeReturned = true;
   } else if(input == 0) {
      doOtherStuff();
      toBeReturned = true;
   }

   return toBeReturned;
}

but this (notice the use of Java's final):

public boolean foo(int input) {
   final boolean toBeReturned;    // no init here
   if(input > 10) {
      doStuff();
      toBeReturned = true;
   } else if(input == 0) {
      doOtherStuff();
      toBeReturned = true;
   } else {
      toBeReturned = false;
   }
   return toBeReturned;
}

By doing so you make your intend clear and this is a godsend for IDEs supporting "programming by intention" (there's no need to "compile" to see potential errors, even on a partial AST, a good IDE can examine incomplete source in real-time and give you instant warnings).

This way you are sure not to forget to initialize your return value. This is great if later on you decide that after all you need another condition.

I do this all the time and even moreso since I started using IntelliJ IDEA (version 4 or so, a long time ago) and this has saved me so many silly distraction mistakes...

Some people will argue that this is too much code for such a simple case but that's entirely missing the point: the point is to make the intent clear so that the code reads easily and can be easily extended later on, without accidentally forgetting to assign toBeReturned and without accidentally forgetting to return from a later clause you may add.

Otherwise, if "conciseness" was the name of the game, then I'd write:

public boolean foo(int a) {
  return a > 10 ? doStuff() : a == 0 ? doOtherStuff() : false; 
}

Where both doStuff and doOtherStuff would return true.

Webinator