Hacker News Reader

Comment by kbelder | Hacker News Reader

I think this is a unpopular opinion, but I consider taking advantage of short-circuiting in boolean operations to be a code smell. At least for me, it seems very fragile and I'd much rather break the conditional logic out explicitly.

But I like to write as much like assembly as I can in every language. Each line of code should do one and only one thing.

View on HN

Short circuiting boolean operators are very useful, and not something I would consider a code smell. They let constructs like this work, which are very common in most codebases:

  if (foo && foo->bar) {
      // whatever
  }

An operator that did not short circuit would have undefined behavior because it might dereference a NULL pointer.

If you're going for "each line of code should do exactly one thing", you'd probably prefer that as

    if(foo) {
      if(foo->bar) {
        //whatever
      }
    }

separating out the null check and the actual conditional. More lines of code and more nesting, yes, but, if you're trying to strictly adhere to a one thing/one line principle, you probably don't care.

Short-circuit 'or' is a little harder to avoid (if you specifically want the short-circuit behavior), since you'd have to duplicate the code in the body of the if. But that doesn't come up as often IME.

Yep, I'd prefer that option, despite its wordiness. Obviously this depends a lot on individual preferences, and in this simple example it really doesn't make a lot of difference.

I imagine the compiler spits out pretty similar code either way. It's more just a recognition of my own limitations... I'm a lot less likely to screw up something up, the more explicitly it's written out.

When I was a younger man, I wouldn't have bothered.

The problem with that code and the reason that it is often considered bad C++ practice is that it is very easy to accidentally move the inner conditional outside of the outer conditional, thus creating undefined behaviour.

Putting the null check in the same line as the usage means that it is much harder to separate them accidentally.

That technique doesn't work (as well) when you want a single "else" block that is used when either of the conditions is false, which I think is pretty common for this sort of conditional

I used to only put short-circuits when I really needed short-circuit behavior, thinking it improved readability. But after getting by the implicit type conversions over and over (as well as nags from every compiler) I just decided it was a bad idea in C++. I'd still do it in a safer language, but in C++ I'm convinced you should just use && even when you only need & for bools.

Reply on news.ycombinator.com