| « Starring The Admin | A Random PHP Script » |
Back in the late-1990s, the Internet Service Provider where Simon C. worked was a mere micro-sized version of what they are today. Their website's original e-commerce system only needed to sell one thing — domain names, and a limited subset of them at that — so the shopping basket and invoicing parts of the system didn't need to be all that intelligent. They simply looped through each item ordered by the customer, displayed the description and prices of each one, and worked out the totals at the end. The whole process was so simple in fact that it made sense to the original developer to write the system so that the shopping cart and invoicing pages shared the same code.
Over time, the ISP grew in size to sell additional products such as new domain types and packages with a multitude of sub-products. Also, as the system grew in size, the site began running slower and slower. This gave Simon a reason to look into ways to improve the efficiency of the shopping basket and invoicing parts of the system.
However, the more familiar Simon got with the code, the harder it was for him to understand why it was allowed to remain in place for over ten years.
They say it takes an entire village to raise a child, but Simon discovered that it took over 16,000 lines of code to process the sale or print an invoice.
How could something like this be? The answer was surprisingly simple. As the company grew, it was simple to add new products. Just add a record to the master product table in the database and then add a new "elseif()" block to the shared code. Same thing went for discounts - toss in another elseif(). T-Shirts? Another elseif()...and so on so that the code was almost entirely made up of hundreds elseif blocks, each of which would need to be evaluated whenever a customer would add or remove an item from their shopping cart. Also, a side-effect of this design was that if someone wanted to change a product's description, for example, they couldn't just edit the database record, they would have to create a new product code and leave the old one in place forever.
Simon figured, My work's cut out for me - it should be easy to improve performance! Just trim out the elseif() blocks for the old items and discounts and I'm done! Ordinarily, Simon's reasoning would be absolutely correct, however, there was a small snag in this plan in the form of several hundred dire warnings.
In looking for candidate code to trim, Simon found that most elseif() blocks heeded the warning DO NOT REMOVE and DON'T TOUCH UNDER PENALTY OF DEATH and the like, just like the following:
// xmas offer 20071206 - 20080103 DO NOT REMOVE, EVER!!
if($rowval['orderitemref'] > 10295755 && $rowval['orderitemref'] < 10373106 && (
(substr($rowval['domainname'], -3, 3) == "com") ||
(substr($rowval['domainname'], -3, 3) == "net") ||
(substr($rowval['domainname'], -3, 3) == "org") ||
(substr($rowval['domainname'], -3, 3) == "biz"))) {
$thestr.='<font face="arial,verdana,helvetica" '
.'size="-2" color="#ff0000"> at special offer price.'
.'<br>Renewal at standard price</font><br>';
}
The reason for this was also simple - as new items, price changes and special offers had come and gone over the years, it had exposed the problem of how the system handles historical invoicing. You see, the important thing about invoices is that they must be immutable. If a customer called up to argue a charge from six months ago, customer service had better be able to pull up that same invoice and it had better match what the customer saw at the time.
What Simon needed was a brilliant plan.
Simon figured he had two options in trying to bring down his goliath. The first option was to refactor the 16,000 lines into a system that would be able to regenerate ten years of invoices flawlessly and spend the rest of his life regression testing while in the meantime, the number of elseif() blocks would inevitably grow.
Alternatively, he could take the quick, dirty, and simple route: print out the historical invoices so that the CSRs could pull up when needed. Well, not print on real paper, but rather batches of PDFs of every historical invoice. This would allow him to simply ditch the old code entirely.
Simon was immensely proud of this solution and presented the idea to management. "Why would we want to change," his boss rhetorically asked, "what we have works already and has been working consistently for the past ten years!"
Defeated, Simon went back to his maintenance work and added yet another elseif() block to represent yet another promo offer.
Re: Immutable Invoices
2009-11-24 09:54
•
by
EleganceIsPerformance
(unregistered)
|
|
Except the problem wasn't just that the code was inelegant. It's that the code was running "slower and slower". The performance problems were presumably why the inelegant code was discovered in the first place. In fact, the TRWTF is that when the manager asked "Why would we want to change?", the answer wasn't simply "Uh...because our customers are complaining that even the simplest transaction takes way too long."
|
Re: Immutable Invoices
2009-11-24 10:29
•
by
Management Victim #654,604,223
(unregistered)
|
|
I WTF'd at the end, when the big bosses back-tracked on their decision to replace the old system with the one Simon was proposing.
Then I realized what the WTF really was: Simon was what we used to call "hobby jobbing". He wasn't acting on management instructions. He wasn't addressing business or operational concerns. He hadn't taken direction in a meeting where his manager (say, Rob, in honor of this) said "Simon, I'm concerned our shopping cart application is getting a bit slow. See what you can do to speed it up." Simon was pursuing a course both self-satisfying and good for the business. But not one that his bosses were aware of and, apparently, would have approved. I suspect a lot of code monkeys out here in WTF-land are thinking that the WTF is Simon not being permitted to do the obviously right thing. "Stupid management, too short-sighted to allow me to solve all their website performance problems." The real WTF is that solving a problem which isn't an actual problem--not hurting productivity or revenue--and in which the solution imposes new problems (like making Customer Service refer to soft-copies of invoices instead of direct lookups) is generally not a good solution. I have to wonder which of Simon's actual tasks he was blowing off to do this journey to the center of the app, the land that code sanity forgot. Yes. It's hideous code, and WTF-worthy in the same sense that a bad accident in a road race is compelling. But if it's functioning, you have to lay the organizational ground very carefully to support "fixing" what isn't broke, from the user perspective. |
|
This looks like a classic case of Code Cancer. The code started out with only a few elseifs, or other bad practices. However, bad practices tend to grow uncontrollably, until physical symptoms began to appear. Once this happens, there are no good options. You are forced to choose between the software development equivalents of chemotherapy or radical surgery.
Like other forms of cancer, early detection and treatment is the best option. |
| « Starring The Admin | A Random PHP Script » |
