Bug story! |
We all strive to write elegant, concise and clear code. One of the tenants of this pursit is the "DRY" principle - don't repeat yourself! But as usual you shouldn't always follow a principle blindly. Take for example the following bug which caused me some problems at customer's premises.
I decided to supervise long running tasks and send a short running ones in parallel, so the concurrency level would be bounded by 2. At two points in the code I have got to check if the next task is a long-runner or not: when a single new task is submitted for processing, an when I need a second, parallel task (which cannot be a long runner again!). As I (true to the agile) first wrote the new task submission code, I defined the:
isLongrunner(filename)method, which looked up if the input data file could be a potential long-runner. This method was used to check if a newly arrived request should be run in parallet to the currently running one. Then the library was tested and the first version was finished.
isLongrunner(filename)method, I should rather use:
isLongrunnerSession(sessFilename)What's the difference? Well, in my client library there is one: the filenames waiting in the input queue will be processed and can be renamed in some specific cases. The isLongrunner() method will check the already processed filenames, as it's passed as paramether to the WorkerThread::process() method*. Unfortunately, in the second case, we are already in the process() method, and are looking directly into the input queue, where the raw file names are to be found! Bugger! And the local test worked, because..., well I can't remember the reason now, probably I didn't test the interface variant (there are two, legacy and modern one - it's a real, living and growing system) the asian client was using. Ooops :(
- If I wasn't so obsessed with code elegance and DRY, I wouldn't be lured into bad reuse, and then the omission to test the right interface version wouldn't have such unpleasant consequences.
- If I'd care about the naming of the methods and its parameters to the point of obsession, I wouldn't settle for the first name which somehow filled the bill. It was sloppy!
* WorkerThread is a part of my portable (as it's Qt-based) worker library, maybe I'll find the time to blog about it. At least the title is already chosen: "Are workers actors?".
No comments:
Post a Comment