The Question:
Does doing if(SomeFunction() == TRUE)
instead of doing if(SomeFunction())
protect against some type of coding error? I'm trying to understand if this is protecting from some hidden land-mine, or if it's the result of someone writing code who didn't quite understand how expressions are evaluated. I understand that if done right, both of these things evaluate the same. Just like if(value == 42)
and if(42 == value)
evaluate the same - still, some prefer the 2nd version because it produces a compiler error if someone typo's the == and writes = instead.
Background: I've inherited some embedded software that was written 4 or 5 years ago by people who don't work here anymore. I'm in the middle of some refactoring to get rid of multi-hundred line functions and global variables and all that jazz, so this thing is readable and we can maintain it going forward. The code is c for a pic microprocessor. This may or may not be relevant. The code has all sorts of weird stuff in it that screams "didn't know what they were doing" but there's a particular pattern (anti-pattern?) in here that I'm trying to understand whether or not there's a good reason for
The Pattern: There are a lot of if statements in here that take the form
if(SomeFunction() == TRUE){
. . .
}
Where SomeFunction() is defined as
BOOLEAN SomeFunction(void){
. . .
if(value == 3)
return(FALSE);
else
return(TRUE);
}
Let's ignore the weird way that SomeFunction returns TRUE or FALSE from the body of an if statement, and the weird way that they made 'return' look like a function invocation.
It seems like this breaks the normal values that c considers 'true' and 'false' Like, they really want to make sure the value returned is equal to whatever is defined as TRUE. It's almost like they're making three states - TRUE, FALSE, and 'something else' And they don't want the 'if' statement to be taken if 'something else' is returned.
My gut feeling is that this is a weird anti-pattern but I want to give these guys the benefit of the doubt. For example I recognize that if(31 == variable)
looks a little strange but it's written that way so if you typo the == you don't accidently assign 31 to variable. Were the guys that wrote this protecting against a similar problem, or is this just nonsense.
Additional Info
typedef enum _BOOLEAN { FALSE = 0, TRUE } BOOLEAN;
Is there a good reason for doing
if(SomeFunction() == true)
instead of doingif(SomeFunction())
No.
If SomeFunction()
returns a result of type _Bool
, then the equality comparison should be reliable (assuming that the evaluation of the result did not involve undefined behavior). But the use of TRUE
rather than true
, and the type name BOOLEAN
rather than _Bool
or bool
, suggest that the result is not an actual _Bool
(available only in C99 or later), but some ad-hoc Boolean-like type -- perhaps an alias for int
.
A value of any scalar type can be used as a condition in an if
statement. If the value is equal to zero, the condition is false; otherwise, the condition is true. If TRUE
is defined as 1
, and SomeFunction()
returns, say, 3
, then the test will fail.
Writing
if (SomeFunction()) { /* ... */ }
is simpler, clearer, and more likely to behave correctly.
Note, for example, that the isdigit()
et al functions declared in <ctype.h>
do not just return 0
or 1
; if the argument is a digit, isdigit()
can (and does) return any non-zero value. Code that uses it is expected to handle it correctly -- by not comparing it for equality to 1
, to true
, or to TRUE
.
Having said that, there might be a valid reason to compare something for equality to TRUE
-- if it matters whether the result is equal to TRUE
or has some other non-zero value. But in that case, using the names BOOLEAN
and TRUE
is misleading. The whole point of a Boolean type is that values are either true or false; there's no "maybe", and if there happen to be different representations of truth, you shouldn't care which one you have.
The guideline I try to follow is:
Never compare a logically Boolean value for equality or inequality to true
or false
(or 0
, 1
, FALSE
, TRUE
). Just test the value directly, with a !
operator if you want to invert the test. (A "logically Boolean" value either is of type _Bool
, or is intended to distinguish between truth and falsehood with no additional information. The latter can be necessary if _Bool
is not available.) Comparison to false
can be safe, but there's no reason to do it; comparing the value directly is still clearer.
And if someone tells you that
if (SomeFunction() == true)
is better than
if (SomeFunction())
just ask them why
if ((SomeFunction() == true) == true)
isn't even better.
See also section 9 of the comp.lang.c FAQ. Its emphasis on pre-C99 solutions is perhaps a bit dated, but it's still valid.
UPDATE: The question asks about a function that returns the value TRUE
of type BOOLEAN
, defined something like this:
typedef enum { FALSE, TRUE } BOOLEAN;
Such definitions were useful in pre-1999 C, but C99 added the predefined Boolean type _Bool
and the header <stdbool.h>
, which defines macros bool
, false
, and true
. My current advice: Use <stdbool.h>
unless there's a serious concern that your code might need to be used with an implementation that doesn't support it. If that's a concern, you can use
typedef enum { false, true } bool;
or
typedef int bool;
#define false 0
#define true 1
(I prefer the first.) This isn't 100% compatible with the C99 definitions, but it will work correctly if you use it sensibly.