What to put in the IF block and what to put in the ELSE block?

Question

This is a minor style question, but every bit of readability you add to your code counts.

So if you've got:

if (condition) then
{
   // do stuff
}
else
{
   // do other stuff
}

How do you decide if it's better like that, or like this:

   if (!condition) then
   {
     // do other stuff
   {
   else
   {
     // do stuff
   }

My heuristics are:

1. Keep the condition positive (less mental calculation when reading it)
2. Put the most common path into the first block

Answer 1

I prefer the first one. The condition should be as simple as possible and it should be fairly obvious which is simpler out of condition and !condition

Answer 2

I prefer to put the most common path first, and I am a strong believer in nesting reduction so I will break, continue, or return instead of elsing whenever possible. I generally prefer to test against positive conditions, or invert [and name] negative conditions as a positive.

if (condition)
    return;

DoSomething();

I have found that by drastically reducing the usage of else my code is more readable and maintainable and when I do have to use else its almost always an excellent candidate for a more structured switch statement.

Answer 3

It depends on your flow. For many functions, I'll use preconditions:

bool MyFunc(variable) {
    if (variable != something_i_want)
        return false;

    // a large block of code
    // ...
    return true;
}

If I need to do something each case, I'll use an if (positive_clause) {} else {} format.

Answer 4

For me it depends on the condition, for example:

if (!PreserveData.Checked)
{  resetfields();}

I tend to talk to my self with what I want the logic to be and code it to the little voice in my head.

Answer 5

If the code is to check for an error condition, I prefer to put that code first, and the "successful" code second; conceptually, this keeps a function call and its error-checking code together, which makes sense to me because they are related. For example:

  if (!some_function_that_could_fail())
  {
     // Error handling code
  }
  else
  {
     // Success code
  }

Answer 6

I agree with Oli on using a positive if clause when possible.

Just please never do this:

if (somePositiveCondition)
else {
    //stuff
}

I used to see this a lot at one place I worked and used to wonder if one of the coders didn't understand how not works...

Answer 7

As a general rule, if one is significantly larger than the other, I make the larger one the if block.

Answer 8

1. put the common path first
2. turn negative cheking into positive ones (!full == empty)

Answer 9

I know this isn't exactly what you're looking for, but ... A lot of developers use a "guard clause", that is, a negative "if" statement that breaks out of the method as soon as possible. At that point, there is no "else" really.

Example:

if (blah == false)
{
    return; // perhaps with a message
}

// do rest of code here...

There are some hard-core c/c++/assembly guys out there that will tell you that you're destroying your CPU!!! (in many cases, processors favor the "true" statement and try to "prefetch" the next thing to do... so theoretically any "false" condition will flush the pipe and will go microseconds slower).

In my opinion, we are at the point where "better" (more understandable) code wins out over microseconds of CPU time.

Answer 10

I think that for a single variable the not operator is simple enough and naming issues start being more relevant.

Never name a variable not_X, if in need use a thesaurus and find an opposite. I've seen plenty of awful code like

if (not_dead) {
} else {
}

instead of the obvious

if (alive) {
} else {
}

Then you can sanely use (very readable, no need to invert the code blocks)

if (!alive) {
} else {
}

If we're talking about more variables I think the best rule is to simplify the condition. After a while projects tend to get conditions like:

if (dead || (!dead && sleeping)) {
} else {
}

Which translates to

if (dead || sleeping) {
} else {
}

Always pay attention to what conditions look like and how to simplify them.

Answer 11

//if you must have multiple exit points, put them first and make them clear
if TerminatingCondition1 then
  Exit
if TerminatingCondition2 then
  Exit

//now we can progress with the usual stuff
if NormalThing then
  DoNormalThing
else
  DoAbnormalThing

Answer 12

Two (contradictory) textbook quotes:

Put the shortest clause of an if/else on top

--Allen Holub, "Enough Rope to Shoot Yourself in the Foot", p52

Put the normal case after the if rather than after the else

--Steve McConnell, "Code Complete, 2nd ed.", p356

Answer 13

Software is knowledge capture. You're encoding someone's knowledge of how to do something.

The software should fit what's "natural" for the problem. When in doubt, ask someone else and see what people actually say and do.

What about the situation where the "common" case is do nothing? What then

if( common ) {
    // pass
}
else {
    // great big block of exception-handling folderol
}

Or do you do this?

if( ! common ) {
    // great big block of except-handling folderol
}

The "always positive" rule isn't really what you want first. You want to look at rules more like the following.

1. Always natural -- it should read like English (or whatever the common language in your organization is.)
2. Where possible, common cases first -- so they appear common.
3. Where possible use positive logic; negative logic can be used where it's commonly said that way or where the common case is a do-nothing.

Answer 14

If one of the two paths is very short (1 to 10 lines or so) and the other is much longer, I follow the Holub rule mentioned here and put the shorter piece of code in the if. That makes it easier to see the if/else flow on one screen when reviewing the code.

If that is not possible, then I structure to make the condition as simple as possible.

Answer 15

When I am looking at data validation, I try to make my conditions "white listing" - that is, I test for what I will accept:

if DataIsGood() then
   DoMyNormalStuff
else
   TakeEvasiveAction

Rather than the other way around, which tends to degenerate into:

if SomeErrorTest then
  TakeSomeEvasiveAction
else if SomeOtherErrorCondition then
  CorrectMoreStupidUserProblems
else if YetAnotherErrorThatNoOneThoughtOf then
  DoMoreErrorHandling
else
  DoMyNormalStuff

Answer 16

You can usually make the condition positive without switching around the if / else blocks.

Change

   if (!widget.enabled()) {
     // more common 
   } else {
     // less common 
   }

to

   if (widget.disabled()) {
     // more common 
   } else {
     // less common
   }

Answer 17

Intel Pentium branch prediction pre-fetches instructions for the "if" case. If it instead follows the "else" branch: it has the flush the instruction pipeline, causing a stall.

If you care a lot about performance: put the most likely outcome in the 'if' clause.

Personally i write it as

if (expected)
{
   //expected path
}
else
{
   //fallback other odd case
}

Answer 18

I always keep the most likely first.

In Perl I have an extra control structure to help with that. The inverse of if.

unless (alive) {
     go_to_heaven;
} else {
     say "MEDIC";
}

Answer 19

If you have both true and false conditions then I'd opt for a positive conditional - This reduces confusion and in general I believe makes your code easier to read.

On the other hand, if you're using a language such as Perl, and particularly if your false condition is either an error condition or the most common condition, you can use the 'unless' structure, which executes the code block unless the condition is true (i.e. the opposite of if):

unless ($foo) {
    $bar;
}

Answer 20

You should always put the most likely case first. Besides being more readable, it is faster. This also applies to switch statements.

Answer 21

I'm horrible when it comes to how I set up if statements. Basically, I set it up based on what exactly I'm looking for, which leads everything to be different.

if (userinput = null){
explodeViolently();
} else {
actually do stuff;
}

or perhaps something like

if (1+1=2) {
do stuff;
} else {
explodeViolently();
}

Which section of the if/else statement actually does things for me is a bad habit of mine.

Answer 22

First of all, let's put aside situations when it is better to avoid using "else" in the first place (I hope everyone agrees that such situations do exist and determining such cases probably should be a separate topic).

So, let's assume that there must be an "else" clause.

I think that readability/comprehensibility imposes at least three key requirements or rules, which unfortunately often compete with each other:

1. The shorter is the first block (the "if" block) the easier is it to grasp the entire "if-else" construct. When the "if" block is long enough, it becomes way too easy to overlook existence of "else" block.

2. When the "if" and "else" paths are logically asymmetric (e.g. "normal processing" vs. "error processing"), in a standalone "if-else" construct it does not really matter much which path is first and which is second. However, when there are multiple "if-else" constructs in proximity to each other (including nesting), and when all those "if-else" constructs have asymmetry of the same kind - that's when it is very important to arrange those asymmetric paths consistently.

   Again, it can be "if ... normal path ... else ... abnormal path" for all, or "if ... abnormal path ... else ... normal path" for all, but it should not be a mix of these two variants.

   With all other conditions equal, putting the normal path first is probably more natural for most human beings (I think it's more about psychology than aesthetics :-).

3. An expression that starts with a negation usually is less readable/comprehensible than an expression that doesn't.

So, we have these three competing requirements/rules, and the real question is: which of them are more important than others. For Allen Holub the rule #1 is probably the most important one. For Steve McConnell - it is the rule #2. But I don't think that you can really choose only one of these rules as a single quideline.

I bet you've already guessed my personal priorities here (from the way I ordered the rules above :-).

My reasons are simple:

• The rule #1 is unconditional and impossible to circumvent. If one of the blocks is so long that it runs off the screen - it must become the "else" block. (No, it is not a good idea to create a function/method mechanically just to decrease the number of lines in an "if" or "else" block! I am assuming that each block already has a logically justifiable minimum amount of lines.)

• The rule #2 involves a lot of conditions: multiple "if-else" constructs, all having asymmetry of the same kind, etc. So it just does not apply in many cases.

  Also, I often observe the following interesting phenomenon: when the rule #2 does apply and when it is used properly, it actually does not conflict with the rule #1! For example, whenever I have a bunch of "if-else" statements with "normal vs. abnormal" asymmetry, all the "abnormal" paths are shorter than "normal" ones (or vice versa). I cannot explain this phenomenon, but I think that it's just a sign of good code organization. In other words, whenever I see a situation when rules #1 and #2 are in conflict, I start looking for "code smells" and more often than not I do find some; and after refactoring - tada! no more painful choosing between rule #1 and rule #2, :-)

• Finally, the rule #3 hase the smallest scope and therefore is the least critical.

  Also, as mentined here by other colleagues, it is often very easy to "cheat" with this rule (for example, to write "if(disabled),,," instead of "if(!enabled)...").

I hope someone can make some sense of this opus...