DevDisasters

Dev Disasters: Just Following Orders

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

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.

About the Author

Mark Bowytz is a contributor to the popular Web site The Daily WTF. He has more than a decade of IT experience and is currently a systems analyst for PPG Industries.

comments powered by Disqus

Reader Comments:

Thu, Nov 3, 2011 Larry Maryland

This validation routine ignores the obvious -- the commas really don't matter, strip them out and validate if it can be parsed. Also check whether the desired/required number of decimal positions are present. private bool validateJunk(string myText, int decimals) { if (string.IsNullOrEmpty(myText) || myText.Length - myText.LastIndexOf('.') > decimals) return false; decimal unused; return decimal.TryParse(myText.Replace(",", string.Empty).Replace("$", string.Empty), out unused); }

Thu, Nov 3, 2011 Peter Vogel Canada

Given a value without a comma (e.g. "$999.99" or "$9999.99") the code correctly flags the value as numeric. Given a value with a comma (e.g. "$9,999.99"), it fails. The problem is in the line if ((commaIndex > 0) && ((i - commaIndex) > decimals)). After the comma is found (and commmaIndex is set), i keeps increasing. It's only a matter of time until (i-commaIndex) is greater than the value passed into the method. Having said that, I'm not sure that I agree with Dave's assessment that the fix is simple. For instance, the test if (commaIndex >= 0) { return (false);} is going to fail when a decimal point is found following the comma. I'm not sure that I agree with Dave's assessment that the fix is safe unless Dave is suggesting using some well tested existing library method instead of this. The simplest, cheapest solution is to tell users to stop entering commas (assuming that no downstream process will fail if commas are omitted).

Wed, Nov 2, 2011 Dana

Since commaIndex was already set to a value when the first comma was encountered the routine returns false when it hits the decimal point.

Wed, Nov 2, 2011

First time to read this column, so perhaps I don't understand the premise. Is this a puzzle for readers to solve? If Dave presented his case to business folks in this fashion, I'm not surpised he was shot down - there's no clarity here.

Wed, Nov 2, 2011

Sorry, I cannot follow this programming language. What was the problem with the code? Was is the comma's being allowed in the dollar amounts when the file is comma delimited?

Add Your Comments Now:

Your Name:(optional)
Your Email:(optional)
Your Location:(optional)
Comment:
Please type the letters/numbers you see above

.NET Insight

Sign up for our newsletter.

I agree to this site's Privacy Policy.