DevDisasters

Dev Disasters: Just Following Orders

A major validation error in the code resulted in a 60 percent failure rate for an expense form.

• By Mark Bowytz
• 11/01/2011

If you worked at the same company as Dave R., and had the misfortune of filing expenses online through the financial department's error-prone Web site, chances are you'd know Judy Long.

It's not that Judy had special powers or abilities above those of her coworkers. Actually, she was just the lucky administrative specialist listed as a go-to contact in every error message of the financial department's expense reporting portal, which upon failure instructed the user to contact Judy and mail her form UC-100B.

Amazingly, despite the fact that 60 percent of all expense reports ended in error and had to be manually processed by Judy, everything continued working for years. That is, until an unforeseen problem arose: After 33 years, Judy decided that it was time to retire.

Naming the Replacement
Naturally, a void would be left by Judy's departure. Every error message that listed her name and contact information needed to be changed. Dave was the lucky developer tasked with hunting down Judy's name in the application and substituting it with that of her replacement.

Almost immediately, Dave knew that finding and replacing Judy's name with someone else's would be a no-brainer. But why not take the opportunity to see why so many people couldn't process their receipts, he thought.

As it turned out, the business logic in the expense application was fairly straightforward; really all that it did was generate and e-mail a comma-delimited file based on user input with some validation.

Curious how something so simple could fail so badly, Dave took a peek at the validation function and was surprised at what he found:

protected bool CheckNumericInput(string myText, int decimals) {
  int commaIndex = -1;
  if (myText[0] != '$') return (false);
  for (int i = 0; i < myText.Length; i++) {
    if ((myText[i] != '0') &&
      (myText[i] != '1') &&
      (myText[i] != '2') &&
      (myText[i] != '3') &&
      (myText[i] != '4') &&
      (myText[i] != '5') &&
      (myText[i] != '6') &&
      (myText[i] != '7') &&
      (myText[i] != '8') &&
      (myText[i] != '9') &&
      (myText[i] != '0') &&
      (myText[i] != '$') &&
      (myText[i] != '.') &&
      (myText[i] != ',')) {
        return (false);
    }

    if ((myText[i] == '.') || (myText[i] == ',')) {
      if (commaIndex >= 0) {
          return (false);
      } else {
        if ((i == 0) || (decimals == 0)) {
            return (false);
        }
        commaIndex = i;
      }
    }

    if ((commaIndex > 0) && ((i - commaIndex) > decimals)) {
        return (false);
    }
  }
  return (true);
}

Prone to Human Error
Of course, the validation approach was ugly and dumb, but why did it fail? Simple: Those individuals followed the directions on the site that said to enter values in $9,999.00 format.

Dave approached management about fixing such an infantile bug as he was replacing Judy's name with that of the new administrative specialist. To his surprise, he was shot down. He tried arguing that it would be a short fix, just one function, and that it was low-risk, but management still objected.

The Judy fix was all that they could afford at the time because their IT support budget was reduced. However, thanks to an expanded personnel budget, they would be able to keep on Judy's replacement -- thus ensuring that receipts would continue to be processed, no matter what the state of the application.