Tuesday, January 13, 2009

Comedy of coding errors

Nerd alert. You will probably enjoy this post if and only if you are a fellow programmer.

There's a fair amount of horrible legacy code here at my new company. It is nobody's fault -- apparently it was written by an intern at another company, then bought by my company. Which is great, because I can loudly ridicule this code without fear of offending anyone. My cubicle-mate is the senior Java developer. Together, the two of us ARE the entire Java team right now; other people here are coding a little bit of Java, but we're the experts and the commerce project is owned exclusively by us.

I ran into a beautifully horrifying bit of code today. First it gathers a list of objects from a table. Then it iterates over each object, seeing whether the object is "authorized"... and then it removes the object from the array. Not only is this inefficient to begin with -- they should have just filtered out the unauthorized objects in the original query -- but they keep rearranging the entire array every time an object is removed. Like this:

1 2 3 4 5
(object 2 is unauthorized)
1 3 4 5

Obviously this runs in O(n^2) time, when it could easily run in O(n) time just by adding a second array.

Wait, it gets worse. I'm trying to fix it, and I realize the same code that kicks out unauthorized objects appears to be in there TWICE... it's iterating over the array twice and doing approximately the same thing each time. I don't know why, but it appears to have something to do with the magic number "50" that keeps showing up in the code. As in:

for (all items) {
if (curItem < 50) {
do one thing
}
}
for (all items) {
if (curItem > 50) {
do almost the same thing, but slightly different
}
}

Dear God, WHY? What does 50 mean? I don't know, the code doesn't give me a clue. You would think 50 is something arbitrary like the number of items displayed per page, but no... only 12 items are displayed per page. ARGH ARGH ARGH ARGH ARGH.

The bright side is that I don't think I've actually had to solve a genuine programming logic puzzle in like this for many months -- I can't remember a single example at DMi. This is fun! When I'm done, the code will run much faster. And there's probably hundreds of examples of this crummy design lurking around, waiting to be fixed.

Yay job security!

12 comments:

  1. nerd.

    I do not understand a thing you said.

    nerd.

    ReplyDelete
  2. LOL
    When I was first learning GWBASIC, there was an odd bug in the math processor in the PC I had to use. I don't remember the specifics anymore, but it was along the lines of 'if a + b = c then goto...'. There were two specific numbers that it just couldn't add right and it would come up with some horribly long integer. To get around that error, I had to not use those numbers ever in my code. Aggravating, but it made for some...entertaining code.

    ReplyDelete
  3. Might be job security, but it gets old pretty bloody quickly.

    Worst similar thing I've seen was the result of many people working on the same bit of code and imparting their own style on each bit. Data comes in in a NULL terminated array (this was C), then copied to a list. Then the code iterates over the list. Then it iterates over the array. Then it leaks the list when it finishes. I think there was a few bugs too (otherwise i wouldn't have come across the code) - not bad for about 15 lines of C. I think it had 5 authors, so at least it had a slight excuse for the weirdness.

    This just reminds me that i'm going back to work Monday after 4 weeks off, and how much i'm dreading it.

    ReplyDelete
  4. Mike Hall3:34 AM

    We have some really, really bad legacy code running on our servers, which I'm busy developing a strategy for replacing. It's a hold over from some really sloppy programmers we used to have. They preferred to write some code and then hack it until it works instead of designing the app properly to begin with. So there is no software design document, no inline comments. Nothing giving a clue on how it is supposed to work.

    I find decyphering what it's meant to do is half the fun. The other half is reading parts to my colleagues so we can have a laugh. As you say, job security. I love problem solving.

    ReplyDelete
  5. I think you should check out http://thedailywtf.com/ for true geek programming bloopers and stories. It's updated daily.

    ReplyDelete
  6. Hopefully it's not as bad as this:

    http://xkcd.com/221/

    XKCD is hilarious!

    ReplyDelete
  7. Thanks for the recommendation, John. I've added the Daily WTF to my "favorites" folder in my RSS reader, and have been reading it regularly since then.

    ReplyDelete
  8. John beat me to The Daily WTF, so I'll just quote from the assortment of one-liners I've gathered over the years: "The faults in bad software can be so subtle as to be practically theological."

    ReplyDelete
  9. Thank you very much, Kazim. I read the first line and immediatly skipped to the next post down, which I enjoyed very much.

    ReplyDelete
  10. If you were god what programing language would you write the universe in?

    ReplyDelete
  11. Voldemort13:
    I'll refer you to The Eternal Flame by Bob Kanefsky, which states unequivocally that God wrote in Lisp.

    ReplyDelete
  12. Anonymous3:58 PM

    Actually you would have more job security be understanding but _not_ optimizing this code. Therefore beeing the only one that can, with a fair amount of work, maintain this beast. :)

    ReplyDelete