Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

In particular, do not default the test for non-zero. For instance, suppose a foo() function returns 0 to indicate failure or a non-zero value to indicate success. Testing for inequality with zero

Code Block
bgColor#ccccff
langc
if (foo() != 0) ...

is preferable to

Code Block
bgColor#ffcccc
langc
if (foo()) ...

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

...

In this noncompliant code example, is_banned() returns zero if false and non-zero if true.

Code Block
bgColor#ffcccc
langc
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();
}

...

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

Code Block
bgColor#CCCCFF
langc
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, 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).

Code Block
bgColor#ffcccc
langc
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();
  }
}

Although the code above will work as intended, it is possible that a future modification will result in the following:

Code Block
bgColor#ffcccc
langc
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();
}

...

The following compliant code sample is preferable for 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.

Code Block
bgColor#CCCCFF
langc
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();
  }
}

...

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

Code Block
bgColor#ffcccc
langc
void login(char *usr, char *pw) {
  User user = find_user(usr);
  if (!strcmp((user->password),pw_given)) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

The code above works correctly. However, to simplify the login code or to facilitate checking a user's password more than once, a programmer can separate the password checking code from the login function in the following way:

Code Block
bgColor#ffcccc
langc
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_user(usr);
  if (!check_password(user, pw)) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

...

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

Code Block
bgColor#ccccff
langc
void login(char *usr, char *pw) {
  User user = find_user(usr);
  if (strcmp((user->password),pw_given) == 0) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

...