This is something I learned the hard way - by many times making the same mistake.
Let's say you have a function AdjustBitrate that accepts one parameter representing new bitrate in kb/s (whatever that may mean). It's prototype is simple:
procedure TBaseClass.AdjustBitrate(newBitrate_kb_s: integer);Due to a changed requirements, you have to redesign this function so that it will take not a new bitrate, but a bitrate increase (relative to the existing bitrate). What do you do?
procedure TBaseClass.AdjustBitrate(bitrateIncrease_kb_s: integer);Wrong! Just the mistake I was talking about. You changed the semantics of a method without breaking existing code!
Just think of what is your next task - you have to find all places where AdjustBitrate is called and fix them to pass bitrate increase as a parameter instead of target bitrate. Without the help of the compiler you'll surely miss one or two. True, Ctrl-Shift-Enter (or Search, Find References) in new IDEs can help a lot, but it is still not perfect. It won't show you other projects by your colleagues, which are unaware of this change in the semantics. Or even worse - what if this is a part of some 3rd party library that many other programmers are using? How will you force them all to update?
The answer is simple - your change must break existing code. In this case, the simplest solution is to rename the method:
procedure TBaseClass.AdjustBitrateBy(bitrateIncrease_kb_s: integer);That's all. The compiler will warn all programmers using your code that AdjustBitrate does not exist and then they'll find AdjustBitrateBy and notice the change in parameters (hope, hope).
Sometimes you're completely sure that no code outside the current project can be using the method you're working on (maybe it is a private method). In that case, I usually rename the method (for example, Add becomes xAdd), fix all call sites for this method (compiler will report errors everywhere I'm trying to use Add), and then rename it back (xAdd -> Add).
[]
I see your point and one thing is for sure: method naming should be as precise as possible so that the names clearly state the code's intention. It's actually a very common thing in refactoring (See Fowler, http://www.refactoring.com/catalog/renameMethod.html).
ReplyDeleteBut, there is another thing here that helps with such situations a lot: unit testing. With that you easily spot such errors, provided you have good code coverage. In the above example, the first thing (before renaming the method) would be to change the test and only refactor the code that reports test failures.
This is something I do all the time :) Another way (not applicable in your example) is to switch the parameters around.
ReplyDeleteAnother thing I do all the time, which has saved my ass on countless occasions, is to put a descriptive comment with 'xxx' (or some other unique string) near code which may not be complete or hasn't been properly tested etc. Essentially it's just keeping important TODO notes but in the code itself near to where the potential issue is.
@gasper: Unit testing is fine, but it is only _unit_ testing. It won't help you when modified code is used in projects outside your control.
ReplyDelete@anonymous: Yep, I also do juggle parameters around. Sometimes.
We usually create a new descriptive method, and add the 'deprecated' keyword to the old method to ensure current code continues to work, but you get compiler warnings as a good way to remind the programmers to update their code...
ReplyDelete