privacyLink.Text = String.Format("<a href=\"{0}\"{2}>{1}</a>", hrefLink, LinkBuilder.ExtractTextFromLink(links[0]));
This caused me a bit of grief this evening. While I am not a fan of building strings like this, it's the way things were and I had to run with it. In an ideal world this would be done with a text or html writer, but I'll be happy with anything short of basic string concatenation.
While refactoring a fairly large application for a web analytics migration, I missed one small but key problem with the line of code listed above. The error given was referencing an array index out of bounds or an undefined array index referenced. Thus I immediately went chasing after a possible problem with the links array being empty or null. This proved futile, and after a few minutes of tracing the flow and checking the history of changes on this file it became apparent that if there was a problem with the array reference, it had been an issue for at least six months.
Google had a cached copy of the page, indexed before my code push, which told me it had definitely been working before I touched anything. I decided a closer look at the String.Format() method call was necessary. In this instance we're expecting a string formatter and an array of one or more strings as additional parameters.
Since this uses late binding of the string parameters into the formatter, the compiler never checks that the correct number of arguments are specified. It would have been nice if the compiler had been designed to parse the formatter and fail the compilation if one too few arguments were provided, but understandably that responsiblity falls on the developer in this case.
There were some key factors in why this bug made it into production before being caught during testing there:
- Partially centralized link building, duplicated in many different portions of the application when a single linkbuilding class would have been ideal. In reality, halfway OOP is often much worse than none at all.
- Code that is not testable with out cost-prohibitive refactoring.
- Limited content and resources with which to test in the staging environment
- The error only occurs when a custom privacy policy link is used, >70% of the time the default is used. Only 5 of well over 100,000 pages were throwing the error.
- Lack of automated tests on production data. We're working to eliminate this particular hindrance, but precious time for such "extras" is always scarce.
I was eventually able to correct the error and push the code up to staging and pre-production to validate the fix. In the end the fix made its way to production before customers became aware of the problem, but these kind of errors are preventable by working to correct the problems listed above.