CodeSOD: The Most Pessimistic Search

Sometimes here at TDWTF, we get code snippets that are immediately obvious in their wrongness. But sometimes, the code only looks mildly inefficient, and it's up to the submitter to let us know how bad it actually is.

Take the following snippet:

logdest=0;if(DbSetFirst(dbptr)!=-1) { while(DbGetNext(dbptr, destptr)!=-1) { if(destptr->physical==DestnameToDest(data[i].Destname)) { logdest=destptr->logical } }

Sure, it doesn't early exit, but depending on the size of the row set, that might be only a minor tweak, right?

Submitter Anders K. tells us this code was typically looping through datasets of around 4,000 rows, and performing successfully with that. When it had to process a 42,000 row batch, however, it took around 20 minutes to update what should have been a once-per-minute status report. That innocent-looking DestnameToDest() is an expensive operation that makes a linear search of a 2048-element array.

The rewritten code, which takes only a few seconds to perform the update?

destptr->physical = DestnameToDest(data[i].Destname);if (DbFind(dbptr, destptr) != -1) { logdest = destptr->logical;}

Our submitter writes: "I challenged a few of my colleagues to come up with a worse implementation, that would still seem reasonable. No one was able to do so." I imagine some of you in the comments will take that as a challenge.

[Advertisement] Scout is the best way to monitor your critical server infrastructure. With over 90 open source plugins, robust alerting, beautiful dashboards and a 5 minute install - Scout saves youvaluable engineering time. Try the server monitoring you'll today.Your first 30 days are free on us. Learn more at Scout.