Page History

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 not default the test for non-zero. For example, testing instance, suppose a foo() function returns 0 to indicate failure, or a non-zero value to indicate success. Testing for inequality with zero:

...

if (foo()) ...

despite the fact convention that C considers 0 to be false0 indicates failure. Explicitly testing for inequality with zero benefits maintainability if foo() is later modified to return -1 rather than 0 on failure.

...

Because most functions only guarantee a return value of non-zero for " true, " the code above is better written by checking for inequality with 0 ("false") as follows.

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();
}

...

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 also common conventions). 

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.

...

In this code, the programmer intended to add error code functionality to indicate the cause of a validation failure. The new code, however, validates any invalid or non-existing user. Because there is not an no explicit test in processRequest(), the logical error is not obvious (the implied "if not validated" seems correct) and seems correct by certain conventions.

Compliant Solution

The following code sample is preferable for code improved maintenance. By defining what constitutes a failure and explicitly testing for it, the behavior is clearly implied and future modifications are more likely to preserve it. If there is a future modification like the one above, it is immediately obvious that the if statement in processRequest() does not utilize the specification of validateUser() correctly.

int 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();
  }
}

...

Comparison functions (such as the standard library function strcmp() function) return 0 if the arguments are equal and non-zero otherwise.

Because many 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 functiona function testing for equality, but not following the same convention as strcmp(), they might instinctively just replace the function name. Also, when quickly reviewed, the code could easily appear to test for inequality.

void login(char *usr, char *pw) {
  User user = find_Useruser(usr);
  if (!strcmp((user->password),pw_given)) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

The code above works correctly.

However, in order to simplify the login code or to facilitate checking a user's password more than once, a programmer might separate the password checking code from the login function. He/she they might do so in the following way.

int check_password(User *user, char *pw_given) {
  if (!strcmp((user->password),pw_given)) {
    return 1;
  }
  return 0;
}

void login(char *usr, char *pw) {
  User user = find_Useruser(usr);
  if (!check_password(user, pw)) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

In an attempt to leave the previous logic intact, he the developer just replaced replaces the strcmp with a call to his their new function. However, doing so would produce incorrect behavior. In the case above, any user which inputs an incorrect password is granted access. Again, two conventions conflict and produce code that is easily corrupted when modified. To make code maintainable and to avoid these conflicts, such a result should never be defaulted.

...

The following approach to using a comparison function for this purpose is preferred. By performing an explicit test, any programmer that who wishes to modify the equality test can clearly see the implied behavior and convention that is being followed.

void login(char *usr, char *pw) {
  User user = find_Useruser(usr);
  if (strcmp((user->password),pw_given) == 0) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

...