by Nick
Friday, July 14, 2006 9:18 AM
I review a lot of code during the day. Some people's code I review after they check-in to source code control, while one person in particular has to show me their changes before they're even allowed to check-in. Whenever she comes to me with her code changes, she always says "I fixed the bug"... and after looking at her code I always have to respond, "Actually, you didn't." This is quickly followed by, "Yes I did... that thing doesn't happen any more." And while that's always true... her solution was to simply comment out the offending code, instead of fixing the offending code, which results in a new bug to replace the one she supposedly fixed.
It's as if the function she commented out was called InsertDefectWithoutDoingAnyRealWork(), when in reality it's called DoSomethingAlmostRightThatStillHasToBeDone(). And then after I tell how her that her solution will cause functioning code to not function any more, and that she needs to go back and really fix the bug... as she leaves, she always says, "OK, but this did work."
No it didn't! Fixing a defect by causing 5 more is not fixing a defect at all! Fixing a defect by commenting out code is not fixing a defect! Every line of code was written for a reason. It may not do exactly what was intended, but there was a purpose to it's being there. So before you simply comment something out, you need to understand that purpose in order to make sure that by the time you're done, that functionality is either deemed to truly not be needed, or has been fixed.
That is when a defect is fixed.