"Joseph M. Newcomer" <newcomer@flounder.com> wrote in message
news:0n1qp3hvu4ogscp1vbk27fk06ri1kfb8ii@4ax.com...
I've seen it far too often. It's one of the things I look for in
client
code, and I've
fixed numerous bugs by removing ==TRUE in comparisons of BOOL values.
I
first started
discovering these by single-stepping through code, seeing nonzero
values
come back, then
seeing some test fail. The test invariably involved testing the
result ==
TRUE, and the
result was != 1. So it is one of the "patterns" I look for when I get
new
code that
"doesn't work, do you think you can fix it?" Sometimes (not too
often)
making the change
fixed the problem they asked me to fix; sometimes (fairly often) it
fixed
problems they
had seen but had never been able to find and fix, and sometimes (all
the
rest of the time)
it fixed problems they didn't know they had, some of which, once
fixed,
made their
customers very happy, but the customers had never reported the
problems!
I've seen these problems using 3rd party libraries, but never with
Windows
API's. I tend to cleanup the code I'm in, so I do in fact remove the ==
TRUE check when I see it, but I think it remains somewhat unknown just
how
many of those bugs it actually fixes. We know it makes things less
error
prone, but we rarely take the time to track down precisely what bugs it
actually fixes, and thus rarely have real proof.
Note that the ==TRUE represents, and always has represented, poor
programming practice,
but the CString implicit cast was documented as working for years. It
was
part of the
design of CString. Now it is considered no longer valid. So I will
stop
depending on it.
I don't consider it hyporcrisy to depend upon a documented feature,
although I do consider
it poor documentation practice to change specified behavior in ways
that
require linear
search of the documentation to find the change.
I've never seen it documented to be guaranteed to work. If it were, it
would be very poor to now not support it.
But boolean-expression-to-boolean-constant comparisons will always be
poor
programming
style. They were poor style in any language, and C doesn't change the
fact that they are
poor style. We saw these in Algol-60, when I started using it in
1967,
and it was poor
style then, and we considered it a mark of an amateur programmer. It
was
poor style in
Bliss, SAIL, FORTRAN, Simula, Ada, and dozens of other languages I've
worked in. Nothing
has changed, and it remains poor style today.
****
There is a difference between poor style and incorrectness.
Incorrectness
is far more serious. We are saying in Windows C/C++ programming, it is
incorrect. A style issue is less important, as style tends to be a
personal
preference.
The problem is that people migrating from other languages (where a bool
can
only have 2 values) to one where BOOL can have many values, what used to
be
a style issue becomes a correctness issue. But it's only incorrect when
the
libraries being used treat a BOOL as having more than 2 values. I think
you
said that the NT codebase actually does return 2 values when a BOOL is
specified; therefore, in modern Windows apps, saying
if ( b == TRUE)
when b is the return value for a Windows API may be poor style, but it
is
still correct.
Yes, it is correct, but what you have is an arithmetic comparison.
etc. which is not what the designers of C intended I think.