Friday, June 29, 2007

Maggot therapy for code

Showing the project owner what I'd done, she had one big question for me. "You started from scratch?" Accompanied, of course, by an incredulous look. So much for subtext.
But let's back up for a second.
So I spent a few months last year building a bunch of pieces of functionality to support a new business need. Late (but under-budget! (since I was the only person working full-time on it)) and hairy as anything, but it more or less works at least some of the time. Hairy enough that when asked "can it do this?" most of the time I roll my eyes and groan and tell them "it's complex and fragile and I'm not sure so I'd guess it'll take me a week" and then sometimes I've already built the functionality. I laugh at Peoplesoft for being Rorschach software and look - I've built some of my own.
As time's gone on, I've gone back in and refactored bits and pieces and have new objects that branch out of the old (that are probably better, honest!). I inherited one subset of these associated objects from a recently-departed coworker.
I've made enough bugs of my own in the past that I worry about other people interacting directly with my stuffs and making me look like a jackass that I've done code audits of the work, so I knew what I was in for.
Loop unrolling.
Bona fide magic numbers. "OK, so there's codes of 3, 4 and 7 that I can apparently pass down from the presentation tier. These are enumerated in the business tier, right? Guess not. Data tier? No. Oh - they drive branching in the stored procedure. Brilliant."
Leaky abstractions. As in the presentation tier sometimes calling business objects, others the data tier, still others the database. Plus handling business logic in the presentation tier.
Code paths in the business tier and database that weren't called in the database - so stored procedures and methods that were essentially dead code.
I'd already given the work a vote of no confidence, so when it was handed to me, I went with my gut - tactical nuke.
But there was that nagging voice in the back of my head. "Good job, Dave - way to go with Not Invented Here Syndrome. Worse, you're exhibiting It Has Been Invented Here, But I'm Too Lazy To Read The Fucking Code So I'll Just Do It All Over Again Syndrome." Couldn't I have spent this week doing something more productive, just fixing the bugs that were logged and moving on, rather than re-doing it pretty much from scratch, virtually guaranteeing that I've introduced more in its stead?
I guess there's that, but given all the code smells... and honestly. Loop unrolling? I've only seen shit like that on The Daily WTF. Who does that shit? Magic numbers? Yow. I've sort of taken an ambivalent stance on them in the past, but when "magic number" means "you have to open up the stored procedure to figure out why the presentation logic passes down this value in this circumstance", I have to scratch my head.
I referred to it as applying a "tactical nuke" when I checked it in, and that too gave me pause. I mean, Joel says you should never completely rewrite it, so you should never completely rewrite it, right?
In medicine (and thank god I don't know this from personal experience), sometimes wounds become necrotic - the tissue's been so damaged that it's no longer living. At times like these, doctors have to become a bit, shall we say, "inventive." I can't think of any other way to describe applying maggots to a human being for medicinal purposes. From the Wikipedia article on maggot therapy (which I'm not linking because fucking eww):

The maggots have three principal actions reported in the medical literature:

  • debride wounds by dissolving only necrotic, infected tissue;
  • disinfect the wound by killing bacteria; and
  • stimulate wound healing.
So calling it a "tactical nuke" was going way overboard. There were bits and pieces in there that worked pretty well. Not the way I would have done them, but I won't argue with working code that works in a reasonably intuitive fashion. So I left the living pieces and excised the suspect ones, the untenable ones.
Two days later, I ultimately think I did the right thing. Two weeks later, after QA's gotten to bang on it, we'll see how confident I am in my skills as Senior Systems Maggot.

No comments: