The form, however, is the same - a list of questions which should be answered by a developer before committing a new code artifact. There are really no particular phases in which to apply these questions, though, like in the Red-Green-Refactor cycle, so I organized them by topic. Feel free to pose these questions to yourself or to a developer you're pairing with anytime you perceive a smell in the code.
Note that many static analysis tools would point out some of this issues, but I think it is better to tackle possible problems early on in the development cycle, at the very moment of the initial design. If you employ TDD, design is performed one step at the time thus leaving you free of aggressive refactoring on the newly introduced concepts: this process converges towards cohesive and loosely coupled classes, which are blueprints for a good object graph.
- Does the class (or interface) name describe what an instance of this class (or interface) does? Usually a name or an adjective plus a noun are good for a class, while an adjective is more appropriate for an interface. Some interfaces have names composed by one or more nouns.
- Does the name contain unnecessary implementation details? Interface and abstract classes should not contain any reference to a particular implementation, but you should analyze this issue in context. For instance, XmlParser is not correct if at least a possible parser implementations does not work with Xml, while for a family of Xml parsers that differ in performance is appropriate. In the same motif, class names should not contain private implementation details of the class which may change, only the class real special trait in respect to the other implementations (e.g. XmlParser, HtmlParser, YamlParser.)
- Is the fully qualified name of the class or interface correct? (also known as: is this artifact placed in the right package or namespace?) You can make a guess based on the number of dependencies that this new artifact introduces.
- Naming conventions and best practices are also valid for method names, parameters, local variables, inline comments.
- Is the name consistent with the rest of the object model? Is it part of the Ubiquitous Language? The more public is the named entity (in the ascending order private, protected, [package where applicable,] public, published), the more important is to get a valid name immediately.
- Should the name be influenced by a standard Ubiquitous Language? This is usually true for design patterns implementation, where leveraging the role names communicates much about the code structure. Other examples of a standard Ubiquitous Language are framework and programming language conventions.
- How many levels of indentation are there in your code? Supposing that the first level is dedicated to methods, other two levels are acceptable, with one (thus two in total) being the norm. Extract Method will help breaking up the complexity in different, orthogonal methods.
- Have you inserted switch constructs, especially similar ones? This structure is usually a smell, which can be refactored with a State or Strategy pattern.
- If-elseif chains or even if-else constructs, when repeated, are equivalent to switch, with the latter being its two-fold substitute.
- Are there new operators mixed with business logic? This is a no-brainer.
- Are there any controversial constructs in the code, such as the static keyword, or goto?
- If the code artifact is a subclass, does it extend the right parent class? If it is an implementation, does it implement the right interface? Check the semantics and analyze the proposition An instance of [entity] is always an instance of [parent], too.
- May a class want to implement only part of this interface? Segregate it in different pieces as much as possible.
- Is the class longer than the standard size for your project? The suggested length varies with the programming language and the particular application, but a long class may be the sign of an imminent Extract Class refactoring.
- Is the class size of the same order of magnitude of other similar implementations?
- How many characters are the most complicated lines long? You may want to introduce intermediate methods or data structures to keep such lines readable. A common rule of thumb is the 80 characters of the original text terminal.
- Method length is also a useful metric. The rule of thumb is a method should be fully visible in a single screen, but this doesn't mean that with a 30'' LCD monitor you should write longer methods. The original screen was 25 lines high, but you may want to extend it a bit for practical purposes and refactor later: extracting local method is one of the simplest refactoring and it has a very limited impact on the rest of the codebase.