Cppcoreguidelines: if (x) vs if (x != 0)

Created on 25 Jul 2017  路  8Comments  路  Source: isocpp/CppCoreGuidelines

At the last editor's meeting we discussed whether we should explicitly recommend one of if (x) or if (x != 0)

The example of if (strcmp(...)) was mentioned as an interesting case.

Most helpful comment

My person preference:

  • If x is / returns an integer: always explicitly compare it to an integer value (preferably to a named constant)
  • If x is / returns a bool/pointer/optional etc,: just use if(x) / if(!x).

Reason is that functions returning an integer often don't have a simple mapping like 0->fail 1->success (just think about windows/Linux API or -- as mentioned -- strcmp) and hence I want to distinguish them from cases where if(x) / if(!x) has the "obvious" meaning of "if x succeeded" or "if x failed" .

All 8 comments

If I may comment on this.
Although the second one is more verbose it matches natural language a bit better: if x is not zero then... compared to the first one, if x then...

It also makes the intent clear to the reader and future maintainer. It will also possibly assist in future maintenance if the type of x changes from perhaps a boolean to a pointer, or a boolean to an integer.

I believe the general preference for the editors was to prefer if (x) to mean if (x != 0) or if (x != nullptr) as this is a long-established C idiom for integral types and pointer types respectively.

The specific concern about if (strcmp(...)) was that it is a known pitfall if the author of the code thought it meant "if they compare equal" when it actually means the opposite. We could special-case this one with a rule to not write that for strcmp specifically.

I thought that SL.str.1 - SL.str.3 kind of rule out the use of naked C-style strings anyway, so it is implied that to use strcmp one should know what he/she is doing.

Even when you know what you are doing functions like std::strcmp are easy to get wrong. I suspect writing the test out the long way doesn't help much if you forget that (for an equality test) 0 means true and anything else means false. If I had to use them more often I might be tempted to simply write a wrapper function:

inline
bool cstrings_are_equal(char const* a, char const* b) { return !std::strcmp(a, b); }

My person preference:

  • If x is / returns an integer: always explicitly compare it to an integer value (preferably to a named constant)
  • If x is / returns a bool/pointer/optional etc,: just use if(x) / if(!x).

Reason is that functions returning an integer often don't have a simple mapping like 0->fail 1->success (just think about windows/Linux API or -- as mentioned -- strcmp) and hence I want to distinguish them from cases where if(x) / if(!x) has the "obvious" meaning of "if x succeeded" or "if x failed" .

@MikeGitb is right on spot.

While strcmp is a concrete example, we shouldn't over-index on it. It is just one example of a long standing C-pattern, that unfortunately is a bug farm: returning an integer with an interpretation of success being 0, positive, or negative values. We do want to recommend against if (x) in those cases.

So, don't build on the idea that C-strings are banned, therefore that form of test is a safe construct.

Bjarne, please consider this in conjunction with #1010

we prefer if(x). see ES.87

Was this page helpful?
0 / 5 - 0 ratings

Related issues

galik picture galik  路  3Comments

TomMettam picture TomMettam  路  3Comments

tamaskenez picture tamaskenez  路  6Comments

Epholys picture Epholys  路  4Comments

qalpaq picture qalpaq  路  5Comments