Let's abuse the if statement!

Some context (ultimately unnecessary for the gist, but I have to vent):

Today was one of those mornings... I casually take a look at a pull request I commented on yesterday. My comment was countered with a dubious explanation and the pull request merged.

I commented on this code:

if (someThing) {
    doStuff();
} else if (someOtherThing && yetAnotherThing) {
    doStuff();
}

No, not a typo - both execution blocks have the same line of code.

My comment was simply a code snippet of the proposed improvement:

if (
    someThing ||
    (someOtherThing && yetAnotherThing)
) {
    doStuff();
}

This is equivalent.

The line-breaks in the expression block are intentional as the 2nd OR member is a bit long. I even added brackets around it although operator priority makes them superfluous.

The author's rebuttal of my proposal was that the if-else-if form was more readable so he'll stick to that one.

More readable?

Really?

Here is a transcript of my thought process when reading that code:

if (someThing) ok, memorised that we're in this execution branch now

doStuff(); ok, were doing stuff when there's some thing

else if oh crap, this has to be simplified

(someOtherThing && yetAnotherThing) argh, grrr, I'll skip reading through the expression and come back to it later, lets see whats in the block

doStuff(); ... wait, lets read that again

doStuff(); that looks like ... the previous if block? lets look at that one again

doStuff(); is that ... the same method call?

It's elementary!

Then I stop and ponder things a bit. Now I'm in Sherlock Holmes mode. This is a puzzle. An enigma. The author's code is actually code, as in - encrypted information.

I lose time on figuring out that this is actually a pattern. The author uses if-else-if instead of if-A-or-B. Wow, I figured it out! How cool! Its so cute that such a simple syntax can be used so creatively.

Then I remember we're not playing games on who can write more clever code. We are building software for money, in a team. As in - more than one of us. That means code must be written with readability in mind.

Don't sweat the small stuff

You might say that the difference is tiny and this is nitpicking.

I disagree.

Using if-else-if so counter-intuitively that it's only understandable by the author (who grew accustomed to it) is extremely counter-productive.

In effect, the author is generating a cumulative cost for the company. Every time someone (other than the author) works with that code they will:

• spend more time to understand it
• have a bigger chance to misunderstand it and add bugs to the code

I agree there's some coding patterns/conventions/habits that are really neither here nor there, but this is not one of them.

This one is plainly bad. Avoid at all costs! (There are no costs, just avoid it.)

Comments

[Craig McIlwrath]

Agreed. The else if hints to me that something else should happen. And then this code shatters that expectation and you have to reprocess.