43

Most satisfying bug I've fixed?

Fixed a n+1 issue with a web service retrieving price information. I initially wrote the service, but it was taken over by a couple of 'world class' monday-morning-quarterbacks.
The "Worst code I've ever seen" ... "I can't believe this crap compiles" types that never met anyone else's code that was any good.

After a few months (yes months) and heavy refactoring, the service still returned price information for a product. Pass the service a list of product numbers, service returns the price, availability, etc, that was it.

After a very proud and boisterous deployment, over the next couple of days the service seemed to get slower and slower. DBAs started to complain that the service was causing unusually high wait times, locks, and CPU spikes causing problems for other applications. The usual finger pointing began which ended up with "If PaperTrail had written the service 'correctly' the first time, we wouldn't be in this mess."

Only mattered that I initially wrote the service and no one seemed to care about the two geniuses that took months changing the code.

The dev manager was able to justify a complete re-write of the service using 'proper development methodologies' including budgeting devs, DBAs, server resources, etc..etc. with a projected year+ completion date.

My 'BS Meter' goes off, so I open up the code, maybe 5 minutes...tada...found it. The corresponding stored procedure accepts a list of product numbers and a price type (1=Retail, 2=Dealer, and so on). If you pass 0, the stored procedure returns all the prices.
Code basically looked like this..

public List<Prices> GetPrices(List<Product> products, int priceTypeId)
{
foreach (var item in products)
{
List<int> productIdsParameter = new List<int>();
productIdsParameter.Add(item.ProductID);
List<Price> prices = dataProvider.GetPrices(productIdsParameter, 0);
foreach (var price in prices)
{
if (price.PriceTypeID == priceTypeId)
{
prices = dataProvider.GetPrices(productIdsParameter, price.PriceTypeID);
return prices;
}
* Omitting the other 'WTF?' code to handle the zero price type
}
}
}

I removed the double stored procedure call, updated the method signature to only accept the list of product numbers (which it was before the 'major refactor'), deployed the service to dev (the issue was reproducible in our dev environment) and had the DBA monitor.

The two devs and the manager are grumbling and mocking the changes (they never looked, they assumed I wrote some threading monstrosity) then the DBA walks up..

DBA: "We're good. You hit the database pretty hard and the CPU never moved. Execution plans, locks, all good to go."
<dba starts to walk away>
DevMgr: "No fucking way! Putting that code in a thread wouldn't have fix it"
Me: "Um, I didn't use threads"
Dev1: "You had to. There was no way you made that code run faster without threads"
Dev2: "It runs fine in dev, but there is no way that level of threading will work in production with thousands of requests. I've got unit tests that prove our design is perfect."
Me: "I looked at what the code was doing and removed what it shouldn't be doing. That's it."
DBA: "If the database is happy with the changes, I'm happy. Good job. Get that service deployed tomorrow and lets move on"
Me: "You'll remove the recommendation for a complete re-write of the service?"
DevMgr: "Hell no! The re-write moves forward. This, whatever you did, changes nothing."
DBA: "Hell yes it does!! I've got too much on my plate already to play babysitter with you assholes. I'm done and no one on my team will waste any more time on this. Am I clear?"

Seeing the dev manager face turn red and the other two devs look completely dumbfounded was the most satisfying bug I've fixed.

Comments
  • 4
    Charming story, indeed.
  • 3
    Your manager sounds like an ass
  • 5
    @Xamenyap

    He was, and partly the reason for being fired.

    Too many "If you want us to change application XYZ...fill out form A...submit paper copies for approval by the manager, head of the department and a vice-president. In our 1-hour process review meeting every 3 months, we'll perform a final review of all the approved suggestions and take them under advisement."

    That works to discourage the 'little people' from wanting improvements in our apps, but when he told that to multiple vice-presidents, more than once...his days were numbered.
  • 1
    @PaperTrail you sure showed those two assholes :D :D bravo man!
  • 1
    Beautiful rant! You earned yourself a subscriber, sir.

    The code example is great! How can these developers even have such a job? The code is fucking ridiculous and so is their ignorant behavior. Shoot them into the sun with a rusty V2!
Add Comment