Musings about games, religion, politics, and other forms of entertainment.

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.

12 comments:

LOLWhen 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.

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.

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.