Any reason for if (function () == TRUE) in C

Question

Question: Does if(SomeFunction() == TRUE) instead of doing if(SomeFunction()) protect against some coding error? I'm trying to figure out if this protects against any hidden surface mine, or if it is the result of someone writing code that doesn't quite understand how expressions are evaluated. I understand that if everything is done correctly, then both. In the same way as if(value == 42) and if(42 == value) evaluate the same thing - some people prefer the second version, because it gives a compiler error if someone types == and writes = instead.

Background: I inherited some of the embedded software that was written 4 or 5 years ago by people who no longer work here. I am in the middle of some refactoring to get rid of many hundreds of line functions and global variables and all this jazz, so this thing is readable, and we can support it in the future. Code - c for pic microprocessor. This may or may not be relevant. There are all kinds of strange things in the code where the screams “didn’t know what they were doing,” but there is a specific model (anti-pattern?) In which I’m trying to understand if there is a good reason for

Template: There are many if statements that take the form

 if(SomeFunction() == TRUE){ . . . } 

Where SomeFunction () is defined as

 BOOLEAN SomeFunction(void){ . . . if(value == 3) return(FALSE); else return(TRUE); } 

Let it ignore the weird way that SomeFunction returns TRUE or FALSE from the body of the if statement, and the weird way that they made the return looks like a function call.

This seems to violate the normal values, which c consider "true" and "false". Like, they really want to make sure that the return value is equal to what is defined as TRUE. It is almost like they create three states: TRUE, FALSE, and "something else". And they don’t want the if statement to be accepted if something else is returned.

The feeling of my feeling is that it is a strange anti-pattern, but I want to give these guys the opportunity to doubt. For example, I admit that if(31 == variable) looks a little strange, but it is written like that, so if you seal ==, you do not accidentally assign 31 variables. There were guys who wrote this, defending against a similar problem, or is it just absurd.

Additional Information

• When I wrote this question, I had the impression that stdbool was not available, but now I see that it is provided by the IDE, it’s just not used in this project. This leads me more to "There is no good reason for this."
• It looks like BOOLEAN is defined as typedef enum _BOOLEAN { FALSE = 0, TRUE } BOOLEAN;
• This is an MPLAB 8.6 development environment.

Answer 1

Is there any good reason to do if(SomeFunction() == true) instead of doing if(SomeFunction())

Not.

If SomeFunction() returns a result of type _Bool , then the equality comparison should be reliable (assuming that evaluating the result is not related to undefined behavior). But using TRUE , not TRUE , and a type name of BOOLEAN , not _Bool or bool , suggests that the result is not the actual _Bool (available only on C99 or newer) but some ad-hoc type of type Boolean - perhaps an alias for int .

A value of any scalar type can be used as a condition in an if . If the value is zero, the condition is false; otherwise the condition is true. If TRUE defined as 1 , and SomeFunction() returns, say, 3 , then the test will fail.

Record

 if (SomeFunction()) { /* ... */ } 

is simpler, more understandable, and more likely to behave correctly.

Note, for example, that the functions isdigit() et al, declared in <ctype.h> , do not just return 0 or 1 ; if the argument is a digit, isdigit() can (and does) return any nonzero value. The code that uses it is expected to handle it correctly - not comparing it to 1 , TRUE or TRUE .

Having said that, there may be good reason to compare something for equality with TRUE - if it matters whether the result is TRUE or has another non-zero value. But in this case, using the names BOOLEAN and TRUE is misleading. The whole point of the Boolean type is that the values ​​are either true or false; there is no “maybe”, and if there were no different ideas about the truth, it doesn’t matter which one you have.

The recommendation I am trying to implement is as follows:

Never compare a boolean for equality or inequality with TRUE or false (or 0 , 1 , false , TRUE ). Just check the value directly with the operator ! if you want to invert the test. (The “logical logical” value is of type _Bool or is intended to distinguish truth and falsity without additional information. The latter may be necessary if _Bool not available.) Comparison with false may be, but there is no reason for this; comparing the value directly is still clearer.

And if someone tells you that

 if (SomeFunction() == true) 

better than

 if (SomeFunction()) 

just ask them why

 if ((SomeFunction() == true) == true) 

not better.

See also section 9 of the comp.lang.c FAQ . His emphasis on C99 preliminary solutions may be a bit outdated, but he is still valid.

Answer 2

Since in C any non-zero value is considered true and only the zero value is false, you should never be compared with one specific definition of the TRUE macro in any case. This is overly specific. The form:

 if( fn() ) 

is the simplest form, but if you prefer to compare with a specific value, then compare only with FALSE in this way:

 if( fn() != FALSE ) // Safer than '== TRUE', but entirely unnecessary 

which will work for all reasonable FALSE definitions, and also if fn() not BOOLEAN . But it remains completely unnecessary.

Personally, for easier debugging, I would prefer:

  BOOLEAN x = fn() ; if( x ) 

In addition to being able to observe the return value in your debugger before entering or skipping a conditional block, you have the opportunity to call x something self-documenting and context-specific, which the function name may not reflect. When servicing, you are more likely to support the variable name than the correct comment (or many comments). In addition, x then available for use elsewhere, and then calls fn() several times (which, if it has side effects or a condition, may not return the same value).

Another problem with user-defined boolean types and values ​​is that definitions can be consistent in general - especially if you use third-party code whose authors also considered it a good idea to define your own using the same character names as yours. If the names are different (for example, BOOL, BOOLEAN or OS_BOOL, for example), when your code interacts with this third-party code, you need to decide which logical type should be used in any specific circumstances, as well as the names TRUE and FALSE may encounter warnings or redefinition errors.

A better approach would be to update the code to use stdbool.h and a real boolean type bool (an alias for the built-in _Bool in C99), which can only have two values TRUE and FALSE . This still does not protect you from the case where fn() not a bool function and returns an integer other than zero or one, but there is a chance that the compiler will give a warning about type mismatch. One of the best things you can do to reorganize legacy code in the event is to set a high warning level and investigate and fix all warnings (and not just through liberal casting!).

Answer 3

It is probably very late, and I'm not 100% sure about PIC, but in AVR some registers are available for use in applications and they provide very fast read and write speeds.

Then the compiler puts Boolean variables there, and then uses SRAM. To stop the compiler from doing this, usually an 8-bit variable is used instead of Boolean, so the compiler uses the available case for the variables that are most used in the application. Thus, the application runs faster, but you should not use Booleans or only when they are used a lot. The only option is to use uint8_t / int8_t, or you can use a makeshift type to act as a boolean being uint8_t / int_t.

In the return section, I always did this on embedded systems. You are returning a single value from a function, and this value may represent a successful operation. Other values ​​are used to display errors or for debugging. Thus, a single variable is used for everyone, and since you want to avoid using a Boolean, you can use an 8-bit variable with full potential.

In general, I would like to emphasize something here, built-in people care about speed and memory much more than programmers. I worked with both people, embedded system engineers generate garbage search code, but it works fast and consumes very little memory. One of the reasons that code looks bad / ugly is because compilers are not as smart as typical compilers used by software developers, and sometimes special tweaks (which may look silly) lead to more efficient assembler. Software developers, on the other hand, spend more time on readability and may not care about code efficiency because the compiler will do most of the work.

If the old embedded engineers were experienced and / or smart, they probably did all of this for a good reason. Especially if they know the hardware well.

Hope this help remains for the rest of the code you go through.