Versions Compared

Key

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

...

Code Block
bgColor#ffcccc

validateUser(User usr)
{
  if(validUsers.contains(usr))
  {
    return 1;
  }

  return 0;
}

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
bgColor#ffcccc

validateUser(User usr)
{
  if(allUsers.contains(usr) == false)
  {
    return 303;  // user not found error code
  }
  if(validUsers.contains(usr) == false)
  {
    return 304; // invalid user error code
  }

  return 0;
}

processRequest(User usr, Request request)
{
  if(!validateUser(usr))
  {
    return "invalid user";
  }

  serveResults();
}

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 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 is preferable for code 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.

...