- Duplicate code - There is no reason to have duplicate code. Try to respect the DRY principle(Don't repeat yourself). As Parnas said best, "Copy and Paste is a design error". Also, coding will become absolutely boring.
- A routine is too long - In OOP, you rarely need a routine longer than one screen. Consider breaking it into multiple routines.
- A loop is too long or too deeply nested - Consider refactoring part of the code as routines, or changing the algorithm. Nested loops are one of the biggest performance penalties.
- A class has poor cohesion - if a class has unrelated responsibilities, consider changing it.
- A parameter list has too many parameters - If you need to pass too many parameters, consider merging them in a cohesive class or rethinking the problem.
- Changes in a class tend to be compartmentalized - this may be a sign that the class should be broken into smaller ones.
- Changes require parallel modifications in different classes - this is a sign that they are tied together. Try cut most of the dependencies. This kind of refactoring can be a real challenge, but it is worthy.
- Inheritance hierarchies need parallel changes - This is a special kind for the problem above.
- Case statements need parallel changes - consider using inheritance with polymorphism instead of case.
- Related data items that are used together are not tied into classes - the first time you code/design, you may overlook some classes. Take your time and create them.
- A routine uses more features of another class than of its own - probably it should be moved into the other class
- A primitive data type is overloaded - For example, you may use int to represent both money and temperature. It is better to create a Money and a Temperature class. By doing so, you will be able to impose custom conditions on the types. Also the compiler will not allow you to mix money with temperatures.
- A class doesn't do much - Maybe it should be merge with another.
- A chain of routines passes tramp data - if a routine takes some data only to pass it to another, you should probably eliminate it.
- A middle man object does nothing - same as above.
- One class is very intimate to another - this works against one of your most powerful complexity management tools: encapsulation.
- A routine has a poor name - In the best case, you can rename it. In the worst, the problem is the design(see a previous post called "About routines"). The name is just a sign. Anyway, take your time to solve this one.
- Data members are public - This is plain wrong. Today, you can use properties in many programming languages, so hiding data behind them is very easy.
- A subclass uses a small percentage of the parent class - usually, this denotes wrong inheritance design.
- Comments are used to explain difficult code - Comments are very good, but creating difficult-to-understand code and commenting it is plain wrong.
- Global variables are used - There are few cases when global variables are the only logical option.
- You need setup/cleanup code before/after calling a routine - try to merge this code into the routine.
- A program contains code that might be needed someday - The only way to write code taking into account future releases is to write it as clear and obvious as possible, enabling quick understanding and modification.
Saturday, October 28, 2006
Reasons to refactor your code
Whenever reading your code, if you stumble upon one of the following cases, it is probably better to stop and refactor that piece of code.