If without else, bad?

Recently while browsing thru old source code I've written in my early programming days I noticed that I often used if's without an else.

//...

if(x == 0)
{
 
}

//...

Now after I have some more programming experience I'm wondering if not using else's should be considered as bad programming practice.

A situation I often encounter in everyday programming is something like implementing an isEmpty() method in a List or Collection object. I can think of three ways such a method could look like:

1.

boolean isEmpty()
{
  return numItems == 0;
}

This only works in trivial cases that are not interesting for this discussion.

2.
boolean isEmpty()
{
  if(numItems == 0)
  {
    return true;
  }
 
  return false;
}

3.
boolean isEmpty()
{
  if(numItems == 0)
  {
    return true;
  }
  else
  {
    return false;
  }
}

I think you can rewrite almost any if-construct so it has an else part. Just like it's shown with the 2nd and 3rd way above.

When I'm programming I always prefer the 3rd way because I think it expresses the logic more clearly than the 2nd way. To me the 3rd way looks just nicer and with a decent editor it is typed just as fast as the 2nd way.

Which way would you prefer? Any aspects I have missed? Are there typical situations where an if-construct can't be rewritten?

ttest
Tuesday, February 24, 2004

I think it's stupid to say that an if without an else is a bad practice.

Because there are two choices after an if (true or false) it doesn't mean that you want to do two separate things.

Sometimes you want to see if a value is present and if yes then do a certain action. If value is not present do nothing. In this case why do you need an 'else'?

Eg:

//////////
If (IsFilePresent() == false) {
    CreateBaseFile();
}

TouchFile();
//////////

Tuesday, February 24, 2004

if (NULL != pMyThing)
{
  pMyThing->someFunction();
}
else
{
  and what would i do in such a case?
}

Tuesday, February 24, 2004

I think the real question is : "Should I short-circuit the exit of  my procedure by using 1 or more return statements in the middle of the code, and specifically at the end of a if block?"

According to Dijsktra, it's not good structured programming (cf "GOTO considered harmful") as it makes the reading the flow of the code more complex (ie: There are more than one exit point in your code, and they can be everywhere).

IMHO
1) is the best when possible
3) otherwise
2) I use it for precondition checking before assert existe'd in Java. 

R Chevallier
Tuesday, February 24, 2004

Depends on situation to situation. There cannot be any hard and fast rule about using or not using else with every if.

As demostrated above, there might very well be situations where you simply do not require an else part.

In the example mentioned in the original post, I would also go with option 3. For the same reasons as mentioned, viz., ease of expression of logic and understanding of the logic at a later stage/by someone else.

T-90
Tuesday, February 24, 2004

boolean isEmpty()
{
    boolean bRetVal = false;
    if (numItems == 0)
    {
          bRetVal = true;
    }
    return bRetVal;
}

Elephant
Tuesday, February 24, 2004

Why bother with a call?
Or at least inline it.

#define isEmpty() (numIterms == 0)

Bathmophobic skier
Tuesday, February 24, 2004

I use it to avoid this:

gboolean
check_thing (thing t)
{
    frob f;
    if (t == NULL) {
        return FALSE;
    } else {
        frob (&f, *t);
        if (f->error) {
            print_error (f->error);
            return FALSE;
        } else {
            if (f->x == t->x) {
                  return TRUE;
            } else {
                  return FALSE;
            }
        }
    }
} 


I find this much more readable:

gboolean
check_thing (thing t)
{
    frob f;

    if (t == NULL) {
        return FALSE;
    }

    frob (&f, *t);
    if (f->error) {
        print_error (f->error);
        return FALSE;
    }
     
    if (f->x != t->x) {
        return FALSE;
    }

    return TRUE;
} 

Mike Schiraldi
Tuesday, February 24, 2004

> According to Dijsktra, it's not good structured
> programming (cf "GOTO considered harmful") as it makes
> the reading the flow of the code more complex (ie: There
> are more than one exit point in your code, and they can
> be everywhere).

I've never really understood this.  I agree with Mike - it makes much more sense to check for errors early and get out of the function than end up with huge chunks of code indented 2 or 3 levels.

Of course, if you add exceptions to the mix then Dijsktra's advice is violated even further but that is another discussion!

Rob Walker
Tuesday, February 24, 2004

How about...

gboolean
check_thing (thing t)
{
    if (t == NULL) {
        return FALSE;
    } else {
        frob f;
       
        frob (&f, *t);
        if (f->error) {
            print_error (f->error);
            return FALSE;
        } else if (f->x == t->x) {
            return TRUE;
        } else {
            return FALSE;
        }
    }
} 

ttest
Tuesday, February 24, 2004

Above is bad code, I say - a function should have one entry, one return.

Bathmophobic skier
Tuesday, February 24, 2004

whats with the

if(xyz != NULL) {
  ... blah ...

?  I prefer

if(xyz) {
  ... blah ...

instead...

i like i
Tuesday, February 24, 2004

I remember being shocked that Microsoft preferred

if (xyz){}

in C/C++ now.  Especially when C#, like most of the non-C world, requires the explicit "!= null."

As for exceptions and the like, it's possible to write good code with or without them, and possible to write crappy confusing code with or without them.  Given a choice, I'll write code with exceptions or early error handling rather than code with indent-itis.

On the other hand, I agree with the people who believe that exceptions made things more confusing.  There are now (n+1)! ways to write bad code.

Of which my least-preferred is bad exception handling with indent-itis, multiple return statements, AND an error condition boolean.  I'd show an example, but they burn my eyes...

Mikayla
Tuesday, February 24, 2004

This is my version.  There is exactly one return true and one return false (and the brackets are in the correct place :-/).  It flows better in my mind to meet the conditions and if it fails fall out until all conditions are met. 

gboolean
check_thing (thing t)
{

    if (t!=NULL)
    {
        frob f;
        frob (&f, *t);

        if (f->error==false)
        {
            if (f->x==t->x)
            {
                return TRUE;
            }
        }
        else    
        {
            print_error (f->error);
        }
    }
    return FALSE;
} 


To the original poster...no if without else is not bad in some cases.  I would be loathe to make such a blanket statement about any programming construct especially the basic constructs.

Billy Boy
Tuesday, February 24, 2004

I agree with Mike and Rob. Programming without else is much clearer to me because the code is more "linear". I'm more at ease dealing with dead-ends ( i.e. return) than with multiple execution paths.

Astrobe
Tuesday, February 24, 2004

In the trivial case I prefer the first way.

> When I'm programming I always prefer the 3rd way because I think it expresses the logic more clearly than the 2nd way.

I'll often put an else as a comment, e.g.

if (IsFilePresent()) {
    CreateBaseFile();
}
//else file already exists

TouchFile();

Christopher Wells
Tuesday, February 24, 2004

Sorry: that should be "if (!IsFilePresent()) ..."

Christopher Wells
Tuesday, February 24, 2004

Does anyone else but me avoid the extra { } on one line if statements.  Such as the following:

gboolean
check_thing (thing t)
{
    frob f;
    if (t == NULL) return FALSE;
    frob (&f, *t);

    if (f->error) {
        print_error (f->error);
        return FALSE;
    }
     
    if (f->x != t->x) return FALSE;
    return TRUE;
} 

I know it's a "bad practice" because people come along and add to your if statements without adding the { }.  But, I always put it all on one line in that case so it's perfectly obvious (yes, I did start out as a VB programmer).  If it doesn't cleanly fit on the line I'm working on (too long) then I split it into a few lines and add the { }.  But I'm just curious if I'm the only one...

Almost Anonymous
Tuesday, February 24, 2004

Almost:

I also avoid {}, but in two lines:
if(XXX)
  YYY
I don't think it's bad practice.

Astrobe
Tuesday, February 24, 2004

"
I also avoid {}, but in two lines:
if(XXX)
  YYY
"

Well I definately think the two lines is bad.  Because it's all tabbed out properly someone can come along and do this:

if (XXX)
    YYY;
    ZZZ;

Well ZZZ will executed no matter what the condition of XXX but on cursory inspection, that's not what it looks like.  As soon as I go to multiple lines, I add the { }.

Almost Anonymous
Tuesday, February 24, 2004

I agree with Almost Anonymous.  Leaving out the brackets can be ok if the "if" statement is all on one line, but as soon as you go to two lines, put in the brackets.  There's just too much of a chance of it getting messed up later.  You avoid any potential foulups with a few simple characters.

Not only do you have to worry about adding a line, but what if you need to test something and end up commenting out the "then" case?  Then you end up with

if (foo)
//    doSomething()

... rest of code ...

All of a sudden, some totally unrelated line of code falls under the "if"

Personally I think that if without brackets is bad practice, but when it is all on one line at least you have less of a chance of fouling it up

As for the multiple return question... we've had this discussion a number of times.  Personally I find it more readable to test preconditions at the beginning of the function and bail out early, so that the rest of the code can assume the preconditions are met without having to be nested in multiple levels of if statements.  This also provides more of a simple linear code flow rather than multiple branchings and harder to follow execution

MikeMcNertney
Tuesday, February 24, 2004

I dislike (1) because it mixes testing a condition with the return. I generally prefer one line of code to do one thing.

I dislike (3) because it violates a straight line path of logic principle. 

(2) is what I do almost everywhere, and when things start looking like (3) I strive very hard to make them look like (2).  I often use a do-while( FALSE ) pattern to break out of conditionals or move blocks of code out into sub-functions to accomplish this.

This may just be my own personal bias but I find that many inexperienced programmers will do (2) and think it's fine coding style.  Then they end up with a ton of nested if-else's everywhere.  Shooting for straight line logic in your code is fundamentally more understandable to humans and is easier to maintain.

Richard Kuo
Tuesday, February 24, 2004

Oops, I mean inexperience programmers will do (3), not (2)!

Richard Kuo
Tuesday, February 24, 2004

"Personally I find it more readable to test preconditions at the beginning of the function and bail out early..."

That's what I prefer too...  that probably leads to my use of the one-line if-statements.  Most of them are returns:

// Test preconditions
if (!IsNumber(x)) return ErrNotNumber;
if (blah) return ErrGeneral;
...
...

// Do work
..
..

Adding all the curly brackets and extra lines just makes the precondition testing section longer and reduces readability.

Almost Anonymous
Tuesday, February 24, 2004

Certain compilers 'may' warn that case 3 is a function that 'may' not return a value.  Although this is a simple case the compiler may notice no explicit return value at the end of the function and whine.

Zekaric
Tuesday, February 24, 2004

Also I sometimes prefer less verbose code as it can be easier to read.  If it's not necessary for an else to be there then no need to use one.  It also leads to less indentation yada...  A matter of personal style I guess.

Zekaric
Tuesday, February 24, 2004

I might take this to a logical extreme...  I'll refactor loops to add continue and break statements to cut down on nesting.  I have a real problem with nesting!  ;)

while (x < 3) {
    if (somecondition()) {
        if (RunFunction()) {
            RunOtherFunction();
            x++;
        }
    }
}

I prefer:

while (x < 3) {
  if (!somecondition()) continue;
  if (!RunFunction()) continue;
  RunOtherFunction();
  x++;
}

(Note that there are no comments in this example; in practice there would be).

Almost Anonymous
Tuesday, February 24, 2004

I prefer

while (x < 3) {
    if (somecondition() && RunFunction()) {
            RunOtherFunction();
            x++;
        }
}

as
Tuesday, February 24, 2004

I knew someone would post that!  I am, with the above exmaple, imagining a case where you would actually need nested ifs.

Almost Anonymous
Tuesday, February 24, 2004

Although, to be honest, I'd still prefer:

while (x < 3) {
    // If doesn't satify conditions, repeat
    if (!somecondition() || !RunFunction()) continue;
    RunOtherFunction();
    x++;
}

Almost Anonymous
Tuesday, February 24, 2004

as, that assumes a specific evaluation order for conditional expressions, but C++ makes no promises on that point.  With this program:

bool A() { cout << "A" << endl; return true; }
bool B() { cout << "B" << endl; return true; }

void main()
{
  cout << "begin test" << endl;
  if (A() || B()) cout << "end test" << endl;
}

Some compilers will output:

begin test
A
B
end test

While others output:

begin test
B
A
end test

And some languages will evaluate just A or B (whichever is evaluated first), because once you've got one true result in a sequence of propositions under the 'or' operator, the entire result will be true.  Common Lisp is like that, for example.

K
Tuesday, February 24, 2004

Sorry, AlmostAnonymous, but I couldn't resist. I still prefer it to your second version.

K, I believe that you are mistaken, and that both C & C++ behave in the same way here - to quote K & R:

"Expressions connected by && or || are evaluated left to right, and it is guaranteed that evaluation will stop as soon as the truth or falsehood is known."

In your example, I would expect the following output from any compiler that did not object to void main():

begin test
A
end test

as
Tuesday, February 24, 2004

I could either way on that example; in fact I probably have code that looks exactly like that.  The longer the code in question, the more I favour my way.

Almost Anonymous
Tuesday, February 24, 2004

as is correct, C and C++ both evaluate left to right and short-circuit, allowing code such as:
if (myPtr && myPtr->testIt()) {
  // do something
}

MikeMcNertney
Tuesday, February 24, 2004

If yall are talking about exit points... I noticed the convention:  // EXIT POINT

But when we're dealing with code where it matters, we're just trying to be less doomed than usual.

Tayssir John Gabbour
Wednesday, February 25, 2004

To prevent indentitis I break up a long function into several other functions.

ttest
Wednesday, February 25, 2004

To Almost: ( if ever someone's still listening)

I should have mentioned that my indent style is

if(maybe)
{
      dothis();
      dothat();
}

In this context a two-lines if() is harmless. Unlike when using:
if(maybe) {
    dothis();
    dothat();
    }

One more reason not to use this indent style; it is evil ( yes, EVIL :) :) :)

Astrobe
Wednesday, February 25, 2004

"To Almost: ( if ever someone's still listening)"

Yeah, I'm still listening.  ;)

Anyway, I really don't think your indent style makes a big difference on the problem I described.  The issue is the mental association between tabs and blocks.  If the line like this:

if (xxx)
  yyy;

The yyy is tabbed out for the same reason that it would be if surrounded by { }.  Stupid people do come along and just add code before / after yyy and assume it's in a block because it's tabbed.

I hate the extra line for the { style.  It's extremely wasteful of vertical space.  Although I do use it for functions/classes (as everyone does) because of the extra spacing.

Almost Anonymous
Wednesday, February 25, 2004

Almost,

In developing my own coding style I follow a "do things one way" principle.  Hence I add braces to all if statements.  That's my rationale for it.  Space schmace!

Richard Kuo
Wednesday, February 25, 2004

My style is all about readibility and simplicity.  I don't like adding unnecessary braces if I don't have to.  It just adds to the visual clutter.

Almost Anonymous
Wednesday, February 25, 2004

Ah, I must have confused that with parameter list evaluation.  It's been a while.

K
Wednesday, February 25, 2004

Parameters are evaluated right-to-left.  When I haven't done C++ in a while and I'm debugging that one always surprises me the first time it comes up.

Once, a long time ago, that actually caused a bug in one of my programs.  I ended up with incorrect results because of that right-to-left evalutation of paramters (and yes, I was doing something weird).  I ended up reversing the parameters to get the correct results.

Almost Anonymous
Wednesday, February 25, 2004

To quote K&R again "The order of evaluation of arguments is unspecified; take note that various compilers differ." I'd be surprised if C++ were different, but I can't face opening Stroustrup at this time of day.

as
Wednesday, February 25, 2004

Certainly parameters can be either pushed left-to-right or right-to-left (or not pushed at all if they're passed via registers), depending on the calling convention, for example depending on whether the function is declared as __cdecl or as __stdcall.

Christopher Wells
Thursday, February 26, 2004

  Recent Topics

  Fog Creek Home