While working on existing code base, I usually come across procedures that contain Abusive use of IF and Switch statements. The procedures consist of overwhelming code, which I think require re-factoring badly. The situation gets worse when I identify that some of these are recursive as well.
But this is always a matter of debate as the code is working fine and no one wants to wake up the dragon. But, everyone accepts it is very expensive code to manage. I am wondering if are any recommendations to determine if a particular Method is a culprit and needs a revisit/rewrite , so that it can broken down or polymophized in an effective manner. Are there any Metrics (like no. of lines in procedure) that can be used to identify such segment of code. The checklist or advice to convince everyone, will be great!
Best Answer
If you want to change this code, first write unit tests that validate it's functionality. Then as you make changes you will have confidence that you haven't broken anything.
There is no hard and fast rule about numbers of lines, but a function with 40+ lines is certainly a hint that it is too long. A function with 2 lines is not too small. Steve McConnell's Code Complete 2 refers to some studies and says that 200 lines is the upper limit for readability.
It's all about cohesion. A single function should do a single task, and all parts of the function should relate directly to performing that task.
Consider cyclomatic complexity. A high number of decision points and deep nesting may indicate too much code in one function.
Also, well-selected function and parameter names make code more readable because they act as a form of documentation. Everything that a function does should be immediately obvious from reading its name and its parameters. If it's hard to explain what a function does in a few comment lines, then you probably have too much code in one function.