Best Practice on IF/ELSE Statement Order

Question

Which is a better practice? (I'm coding in .Net if that makes a difference)

IF condition = true THEN
   ...true action--even if rare...
ELSE
   ...action
END IF

or

IF condition = [most common condition] THEN
   ...most common action....
ELSE
   ...least common action
END IF

Answer 1

Go with the most readable version for your specific case, and by the way, don't compare a boolean expression to true and false. Use condition and Not condition (!condition in C#.)

if (condition == true) // bad
if (condition) // better

Answer 2

Use whichever makes the code easier to read. This is usually the second of your options.

Edit

In a lot of cases, it depends on what you're trying to achieve, for example, if you want to check that a connection started correctly:

Connect()
if connected then
    SendString("Hello!")
else
    FlagConnectionFailed()
endif

Whereas if you want to catch an error:

' Just about to send something
if not connected then
    FlagConnectionLost()
    return
endif
SendString("Still connected!")

But you may even want to:

Disconnect()
if not connected then
    return "Complete"
else
    FlagConnectionDisconnectFailure()
endif

(I'm not a VB programmer, so the syntax above is largely made up!)

Answer 3

According to Steve McConnell, author of Code Complete, you should

"Put the case you normally expect to process first. This is in line with the general principle of putting code that results from a decision as close as possible to the decision...[putting the normal case after the if] puts the focus on reading the main flow rather than on wading through the exceptional cases, so the code is easier to read overall."

Code Complete, 2nd Edition, pages 356-357.

Answer 4

First of all, you shouldn't compare to boolean values, this is, do

if condition then

instead of

if condition = true then

About your question, it depends on the natural variable names, IMO.

For example, if you are creating a client that needs to check if it's connected (the most common case)

if connected then
    //Proceed
else
    //Throw error
end if

Or, if you are creating a different program, where you have a variable, say, retrieved and you want to know if the content has been retrieved

if not retrieved then
   //Error
end if

Do not do

if retrieved then
else
    //Error
end if

Answer 5

As others have said readability is generally more important. However, readability means different things to different people.

For me, it typically means arranging the if statement so that the shorter action (in terms of lines of code) comes first, so that if the statement is near the bottom of the window I'm more likely to see the "Else" on screen.

For others, placing a "Not" in the condition can really throw them, and so they'll prefer to list it so the the If condition is always as positive as possible.

Answer 6

Generally, I would always put the true clause first. Something like this, for me, obfuscates meaning:

If not something Then
  'do something 1
Else
  'do something 2
End If

This results in a double-negative, much better to write it like this:

If something Then
  'do something 2
Else
  'do something 1
End If

I believe this recommendation comes from code complete. A great book well worth reading

http://www.cc2e.com/

If you're going to have more than one else then it might be better to consider a case statement.

Answer 7

The better practice is the second option - most common action first.

It makes it easier to read the code as you are not distracted by the code for the less used/exceptional case.

Answer 8

In the final assembly/machine code, it does make a difference. The statement which is most likely to be executed is done in the path without the branch. This way, the pipeline isn't broken causing valuable cycles to be lost.

I have no clue if the compiler leaves your if then statement order intact and this way forces the assembly to take this optimized route.

I have read that visual studio 2008 (when it was announced) would have an optimization functionality where the compiler adds measurements at branches and then during runtime measures how often a certian path is taken. Then in subsequent recompiles the most optimal code path is preferred.

I have no clue if this feature ever made it past the 'design/academic phase'

Answer 9

If the most common case isn't the simplest to express, you might have an opportunity for re-factoring

One useful re-factoring I've found:

if (a.getFoo() == 1 && a.getBar() == 2)

can be re-factored to

if (a.isFooBar())

In come cases something nasty like this,

if (!(fooSet.contains(a.getValidFoo())))

could be

if (a.hasInvalidFoo(fooSet))

This can make option 1 also be option 2 by simplifying the evaluation of the most common condition.

Answer 10

You have received some pretty good answers already. I'm going to approach the question from a different angle.

First, as far as performance goes it may not matter as much as you think in modern CPUs. That's because they use a feature called branch prediction in which the CPU attempts to predict the most likely direction the code will take. Of course, I still agree that you should place the most likely branch at the top if performance is your main concern.

Second, I prefer readability over trivial performance enhancements. In most cases the benefit of readability outweigh those of performance.

Third, use guard clauses when possible. It makes the code more readable.