What's the most unsound program you've had to maintain?
- by Robert Rossney
I periodically am called upon to do maintenance work on a system that was built by a real rocket surgeon. There's so much wrong with it that it's hard to know where to start.
No, wait, I'll start at the beginning: in the early days of the project, the designer was told that the system would need to scale, and he'd read that a source of scalability problems was traffic between the application and database servers, so he made sure to minimize this traffic. How? By putting all of the application logic in SQL Server stored procedures.
Seriously. The great bulk of the application functions by the HTML front end formulating XML messages. When the middle tier receives an XML message, it uses the document element's tag name as the name of the stored procedure it should call, and calls the SP, passing it the entire XML message as a parameter. It takes the XML message that the SP returns and returns it directly back to the front end. There is no other logic in the application tier.
(There was some code in the middle tier to validate the incoming XML messages against a library of schemas. But I removed it, after ascertaining that 1) only a small handful of messages had corresponding schemas, 2) the messages didn't actually conform to these schemas, and 3) after validating the messages, if any errors were encountered, the method discarded them. "This fuse box is a real time-saver - it comes from the factory with pennies pre-installed!")
I've seen software that does the wrong thing before. Lots of it. I've written quite a bit. But I've never seen anything like the steely-eyed determination to do the wrong thing, at every possible turn, that's embodied in the design and programming of this system.
Well, at least he went with what he knew, right? Um. Apparently, what he knew was Access. And he didn't really understand Access. Or databases.
Here's a common pattern in this code:
SELECT @TestCodeID FROM TestCode WHERE TestCode = @TestCode
SELECT @CountryID FROM Country WHERE CountryAbbr = @CountryAbbr
SELECT Invoice.*, TestCode.*, Country.*
FROM Invoice
JOIN TestCode ON Invoice.TestCodeID = TestCode.ID
JOIN Country ON Invoice.CountryID = Country.ID
WHERE Invoice.TestCodeID = @TestCodeID AND Invoice.CountryID = @CountryID
Okay, fine. You don't trust the query optimizer either. But how about this? (Originally, I was going to post this in What's the best comment in source code you have ever encountered? but I realized that there was so much more to write about than just this one comment, and things just got out of hand.) At the end of many of the utility stored procedures, you'll see code that looks like the following:
-- Fix NULLs
SET @TargetValue = ISNULL(@TargetValue, -9999)
Yes, that code is doing exactly what you can't allow yourself to believe it's doing lest you be driven mad. If the variable contains a NULL, he's alerting the caller by changing its value to -9999. Here's how this number is commonly used:
-- Get target value
EXEC ap_GetTargetValue @Param1, @Param2, OUTPUT @TargetValue
-- Check target value for NULL value
IF @TargetValue = -9999
...
Really.
For another dimension of this system, see the article on thedailywtf.com entitled I Think I'll Call Them "Transactions". I'm not making any of this up. I swear.
I'm often reminded, when I work on this system, of Wolfgang Pauli's famous response to a student: "That isn't right. It isn't even wrong."
This can't really be the very worst program ever. It's definitely the worst one I've worked
