current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 6 down vote favorite

I have an if/else if structure that on some cases does nothing. From the code I have seen in my career, the "empty" case is normally left out. But when I wrote my code, it just doesn't feel right to leave this case out, but to show it as a real case that simply does nothing. For example, in a case where something is done when a number is below 5 and above 10, but in between nothing is done:

int a = 4
if(a < 5) {
  do something
} else if(a >=5 && a <= 10) {
  // do nothing
} else if(a > 10) {
  do something else
}

The reason I thought this is a better option is because:

  1. The is how I though about the problem in my mind.
  2. This shows the reader of the code that I thought of all the possibilities and didn't forget one by mistake.

So I was wondering if this convention is either accepted by the programming community or it is shunned upon.

share|improve this question
    
possible duplicate of Elegant ways to handle if(if else) else and of Are too many if-else statements for validation bad? –  gnat Oct 4 '13 at 11:27
    
In your example it's ok, but in other cases where a is not an int but for instance a property, the mere checking of a multiple times can have sides effects. –  Pieter B Oct 4 '13 at 11:29
    
What about if a is less than or equal to zero? Is this supposed to only handle positive numbers? –  eidsonator Oct 4 '13 at 15:44
    
@gnat: Your 'duplicate detector' seems to have malfunctioned here. The only relation between this Q and the two you mentioned as possible duplicates is that they concern if-statements. –  Bart van Ingen Schenau Oct 8 '13 at 9:49
    
@BartvanIngenSchenau I believe these are dupes. And it's interesting how "conveniently" you omit else-statements in your analysis of the questions –  gnat Oct 8 '13 at 10:23
show 1 more comment

2 Answers 2

up vote 9 down vote accepted

You are not simply checking the value of a for the sake of it. It probably means something.

Thus I would prefer to see:

Boolean sell = a < 5;
Boolean buy = a > 10;
Boolean error = a >= 5 & a <= 10;
// or even: 
Boolean error = !sell & !buy;
/* ^ (that's single source principle: if 5 changes to 6 one day, 
 * you don't have to remember to change it in two places) */

And then:

if (sell) {

} else if (buy) {

} else if (error) {

}

Isn't it clearer?

Furthermore, could a be both less than 5 and greater than 10??

Rather not, so those elses are clearly redundant. (Unless you change the value of a within one of your if blocks, mind you, If you only evaluate a once, however - as in my example - you don't have to worry about it).

Hence:

Boolean sell = a < 5;
Boolean buy = a > 10;
Boolean error = !sell & !buy;

if (sell) {

} 
if (buy) {

} 
if (error) {
    // handle error
}

This is way clearer and more flexible in my opinion - if some conditions are no longer mutually exclusive, as it can happen, you won't need to do much refactoring at all.

share|improve this answer
    
Very interesting idea. Only thing I don't like about it is that it makes the code longer, which makes it "harder" to read. But otherwise it is very elegant. –  vainolo Oct 4 '13 at 12:09
3  
@vainolo yes it's longer by three lines, but I think it actually improves the readability (since your conditions are given meaningful names) and so the trade off is worth it. Of course ultimately it comes down to taste –  Konrad Morawski Oct 4 '13 at 12:17
4  
When the difference in efficiency is negligible as in this case, readability is most important. Simplifying conditionals by extracting and naming their components is a great way to improve readability. –  Mike Partridge Oct 4 '13 at 12:29
1  
I like a series of if-else-if-else and virtually always have an else at the end. Similarly for a switch-statement, I virtually always have a default case. I think that it's just a good idea to always consider every case, and if a case is "impossible" then have an assert statement there (if not throw an exception), and to be sure to turn on assertions for debugging and for testing. –  Kaydell Oct 5 '13 at 4:46
add comment
up vote 11 down vote

I would avoid to define the do nothing case explicit by giving the intervall. I would use an else-branch for that

int a = 4
if(a < 5) {
  do something
} else if(a > 10) {
  do something else
} else {
     //This should mean: (a >=5 && a <= 10)
     // do nothing

     // Maybe log something here 
     // or even add an assertion while development to ensure that the 
     // branch is only reached in expected cases:
     assert (a >=5 && a <= 10) : "I expected 'a' to be 5<=a<=10, but a is:"+a;
}
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.