Have done several code reviews in past and found following top 5 code smells common across most of these code having code quality issues:
- Large Class: The classes were found larger enough due to lack of developers’ understanding on one of the primary coding principles which is “Single Responsibility Principle” (SRP). These classes use to get larger due to various methods serving unrelated functionality in the same class.
- Long Method: The methods have been found longer due to several reasons such as following:
- Several block of code serving unrelated/multiple functionality within the same method. This is primarily due to lack of SRP concepts with the developers.
- Several conditionals within the same method. This is found very often within the method which are long enough. This may be attributed to the lack of knowledge on McCabe code complexity vis-a-vis SRP concepts.
- Several Method Parameters : Then, there are several instances I came across where it was seen that method is passing several parameters to each other to interact/call each other. And, a change to one of the parameters in the parameter list use to change several method signature.
- Literal Constants Used Everywhere: Several times, it is seen that developers (primarily newbies) use the literal constants (mostly numbers) with definite meanings using them as it is, without assigning a proper named constant variable to it. This degrades the code read-ability and understand-ability.
- Vague Method Names: Many a times, method names have been found to look like following which impacts on code read-ability and understand-ability:
- Vague names without any meaning
- Technical names without any reference to problem domain related names.
Top 6 Refactoring Patterns to Deal with Above Code Smells
Following are top 6 refactoring patterns which could help you resolve 80% (remember 80-20 rule) of code quality issues and become a better developer:
- Extract Class/Move Method: As mentioned above, code smell such as large class serving functionality that should be done by two or more classes should be split/extracted into appropriate number of classes. In these new classes, the appropriate fields and methods from the old classes should be moved. Additionally, the classes are also found to be long at times because of presence of methods which serves functionality which are used by other class and tends to be the member method of that class. Such methods should also be moved to such type of appropriate class.
- Extract Method: Code smell such as long method as mentioned above could be dealt by extracting code from old method to one or more new methods.
- Decompose Conditional: many a times, method found to be long is seen to consist of several conditional statements (if-else). These conditionals should be extracted and moved into separate methods. This does increase the code read-ability & understand-ability to a great extent.
Introduce Parameter Object/Preserve Whole Object: Several parameters passed to methods has been one another common observations I have come across while doing code review. The problem has been primarily related to change in code if there has been a need to add or remove one of the method parameters from the involved methods. In this scenario, it is suggested to group related parameters as object (Introduce Parameter Object) and let methods pass these objects rather than each of the parameters.
- Replace Magic Number with Symbolic Constants: For literal constants that have meanings and that are used everywhere, should be assigned a named constant variable. This enhances the code read-ability and understand-ability to a great extent.
- Rename Method: As mentioned above under the code smells, the vague method names impacts the usability of the code. These vague names should rather be given meaningful names which may relate to business domain related terminologies and help developer understand the code in the business context. This is quite a skill and requires developers to collaborate with business analysts to properly understand the business requirements that is met with the code. Interestingly, this refactoring pattern looks pretty simple to understand, but however, gets ignored quite often by many developers given the fact that its mention is made in IDEs such as Eclipse under refactor menu link.