I really dislike the term "code smell". Nothing wrong with the concept, but it tends to be used to make half-baked, patronizing remarks about other people's code. And when you try to argue there's nothing wrong with a particular piece of code, you get hit back with "it's bad and you just don't know it, and I can't explain it but it just is".

If you can't verbalize your reasons, it doesn't belong in a code review -- otherwise it's preference. And that's where people get confused about what a "code smell" is really supposed to be.

The article does not suggest an alternative. If I consider any typical real-life case where I would use else, trying to finagle the code into not needing else would make it smell considerably more.

Else is a common name in my part of the world and capitalizing the word makes the connection more explicit.

i agree about the "first smell", the inverted condition, and fairly recently started doing the reverse:

  $x=a

  if (condition) then $x=b

I like this, I think it's easier to read.

I don't care that its not the most efficient way to do things, programming languages exist to make things easier for me.

    $x=a  
    if (condition) then $x=b

If you think of programs as logic then I see the lines above as a logical expression written in CNF conjunctive normal form, where each line is a clause.

whereas this...

    $x = condition ? b : a;

...is like a nested logical expression.

And breaking the program down into clauses makes it easier to understand, for humans and computers.

A proposed solution from the article:

> Use "if (condition()) { ifBlock(); } if (!condition()) { elseBlock(); }" -- assuming that ifBlock() can't possibly change the result of condition().

What?? That seems terrible to me - just asking for `condition` to be updated to be modifiable by `ifBlock`, or for some completely undebuggable race condition to occur.

My team has an ongoing style war about guard clauses:

  if foo:
    bar()
  baz()

vs

  if foo:
    bar()
  else:
    baz()

Consensus seems to favor the first option but I prefer the second because it helps me keep in mind the conditions that lead me to `baz()`. Otherwise I have to scan up the whole function to figure out the restriction `!foo`.

That’s not a style issue, those are two different things.

If foo is true, bar() and baz() are both executed in your first scenario. Only bar() is executed in the second.

Oh whoops you're right, Friday brain. The first conditional should have a `return` in it.

See also “Else Considered Helpful”:

https://wiki.c2.com/?ElseConsideredHelpful

if you really can’t abide early returns.

This is a high-quality site, though.

It’s just an abstraction for the idea of “this pattern tends to signal a problem”. The same way humans have learned that certain smells signal spoiled meat or rotten eggs. Sometimes those smells can safely be ignored, like when eating Limburger cheese or durian fruit.

Code smells themselves aren’t problems, they’re just useful track marks that might indicate a problem is lurking nearby. Worth looking into and making the decision yourself.

Poor patterns elicit physical repugnance in some individuals, so the term is only more correct in those cases.

The term “smell” over something more concrete like “rotten” is quite perfect as what smells for me might not smell for you; whereas if something is rotten there’s little dispute.

Additionally, a code smell can describe code that feels wrong, but you can’t quite put your finger on it. Similar to catching a whiff of a new smell when out. You can’t quite discern where it came from, but it’s there.