Perform explicit tests to determine success, true/false, and equality to improve the readability and maintainability of code and for compatibility with common conventions.
In the case that the return value will never change, an explicit test is still preferable to clearly communicate the numeric, rather than boolean, value of the test.
In particular, do Do not default the test for non-zero.
Essentially,For example, testing for inequality with zero:
Code Block |
---|
#define FAIL 0 if (foo() != FAIL0) ... |
is preferable to
Code Block |
---|
#define FAIL 0
if (foo()) ...
|
despite the fact that C considers 0 to be false. The first will be beneficial if it is later decided that a failure return should be -1 rather than 0.Performing explicit tests to determine success, true/false, and equality makes code both maintainable and compliant with common conventions.
In the case that the return value will never change, an explicit test is still preferable to clearly communicate the numeric, rather than boolean, value of the testExplicitly testing for inequality with zero benefits maintainability if foo()
is later modified to return -1 rather than 0 on failure.
This recommendation is derived from and considers the implications of the following common conventions:
(1) Functions return 0 if false, but only and non-zero if true [1].
(2) Function failures can typically be indicated by one of the following return values: -1, a non-zero number.
(3) Comparison functions (such as the standard library function strcmp()) return 0 if the arguments are equal and non-zero otherwise.Here is a typical discussion among programmers on the subject of convention (1).
Noncompliant Code Example
In regards to (1):The following is fairly common yet ignores the convention that most functions in C only guarantee a non-zero return value to indicate True/Yes/etc..this noncompliant code example, is_banned()
return 0 if false, and non-zero if true.
Code Block | ||
---|---|---|
| ||
LinkedList bannedUsers; int is_banned(User usr) { int x = 0; Node cur_node = (bannedUsers->head); while (cur_node != NULL) { if(strcmp((char *)cur_node->data, usr->name) { x++; } cur_node = cur_node->next; } return x; } void processRequest(User usr) { if(is_banned(usr) == 1) { return; } serveResults(); } |
The call function processRequest()
ignores the convention that most functions in C only guarantee a non-zero return value to indicate "true".
If a banned user is listed twice, he gets the user is granted access. Although checkis_
user banned()
follows the common convention of returning non-zero for true, processRequest
only checks for equality with 1.
...
Code Block | ||
---|---|---|
| ||
LinkedList bannedUsers; int is_banned(User usr) { int x = 0; Node cur_node = (bannedUsers->head); while(cur_node != NULL) { if (strcmp((char *)cur_node->data, usr->name) { x++; } cur_node = cur_node->next; } return x; } void processRequest(User usr) { if (is_banned(usr) != 0) { return; } serveResults(); } |
Noncompliant Code Example
In regards to (2):this noncompliant code example, function failures can typically be indicated by one of the following return values: -1, a non-zero number.
Although failures are frequently indicated by a return value of zero (which C considers to be false), there are common conventions that may conflict in the future with code where the test for non-zero is not explicit. In this case, defaulting the test for non-zero welcomes bugs if and when a developer modifies foo to return an error code or -1 rather than 0 to indicate a failure (all of which are common conventions).
Code Block | ||
---|---|---|
| ||
int validateUser(User usr) { if(listContains(validUsers, usr)) { return 1; } return 0; } void processRequest(User usr, Request request) { if(!validateUser(usr)) { return "invalid user"; } else { serveResults(); } } |
The code above will work as intended. However, it is very feasible that a future modification will result in the following.
Code Block | ||
---|---|---|
| ||
errno_t validateUser(User usr) { if(list_contains(allUsers, usr) == 0) { return 303; // user not found error code } if(list_contains(validUsers, usr) == 0) { return 304; // invalid user error code } return 0; } void processRequest(User usr, Request request) { if(!validateUser(usr)) { return "invalid user"; } serveResults(); } |
...
Code Block | ||
---|---|---|
| ||
errno_t validateUser(User usr) { if(list_contains(validUsers, usr)) { return 1; } return 0; } void processRequest(User usr, Request request) { if(validateUser(usr) == 0) { return "invalid user"; } else { serveResults(); } } |
Noncompliant Code Example
In regards to (3):Comparison functions (such as the standard library function strcmp()) return 0 if the arguments are equal and non-zero otherwise.
Because comparison functions (like strcmp) return 0 for equality and non-zero for inequality, they can cause confusion when used to test for equality. If someone were to switch the following strcmp call with an equals function, they might instinctively just replace the function name. Also, when quickly reviewed, the code could easily appear to test for inequality.
Code Block | ||
---|---|---|
| ||
char *a,str1; char *bstr2; if (\!strcmp(astr1,b str2)) { return "strings are equal"; } |
...
The following approach to using a comparison function for this purpose is preferred.
Code Block | ||
---|---|---|
| ||
\#definechar STREQ(str1, str2) (strcmp((str1), (str2)) == 0) char *str1,*str1; char *str2; if ( strcmp(str1, str2) == 0) ) { return "strings are equal"; } |
...
Risk Assessment
Code which does not conform to the common practices presented will be difficult to maintain. Bugs may easily arise when modifying helper functions which evaluate true/false or success/failure. Bugs may also easily arise when modifying code that tests for equality using a comparison function that obeys the same conventions as standard library functions like strcmp.
...