- Comments – public methods should have comments. In the same time too many comments is bad and means that the code is unreadable without the comments. The code should be clean enough to avoid comments inside of the methods. Also, you should to avoid the comments which saying “if you update this, then update the next…”.
- Duplicated code – is when you have blocks of code that are similar but with slight differences. These blocks appear in multiply places of the system.
- Long method – self explained, you don’t want to have long methods. Having a long method can sometimes indicate that there is more occurring in that method than should be. Or it’s more complex than it needs to be.
- Large class – its referencing the “God classes”/”Black hole classes” which are getting bigger and bigger. They normally start as a regular size class. But as more responsibilities are needed, these classes seems like the appropriate place to put the responsibilities. But as it going to have more responsibilities, then it tend to attract more responsibilities, hence the class going to be larger and larger and keeps attract more and more responsibilities. To avoid this, you need to be explicit about the purpose of the class and keep the class cohesive, so it does one thing well.
- Data classes – too small classes are also an issue, it calls “Data classes” – are classes that contain only data and no real functionality. Generally these classes would have getter and setter methods, but not much else. (But I agree that POCO/POJO can be useful, no silver bullet, need to see the context to get the right design).
- Data clumps – are groups of data appearing together in the instance variables of the class or parameters to methods. The fixing of the code smell can lead to the data classes code smell, the class should have something more than just data (some functionality).
- Long parameter list – difficult to use correctly and increasing the chances that something is going wrong. To fix it, need to introduce parameter objects.
- Divergent change – occur when you have to change a class in many different ways, for many different reasons. This closely related to the Large class code smell which have too many responsibilities which may to be changed in variety of ways. So, poor separation of concerns is a common cause of divergent change. it will be nice, if your class only had one specific purpose as it should. Even a class may be extracted into few classes based on different responsibility (the large class delegate responsibilities to the extracted classes).
- Shotgun surgery – this is the commonly occurring smell. A change in one place requires you to fix many other areas in a code as a result. This could happen when you are trying to add a feature, adjust code, fix bugs or change algorithms. Ideally the changes should be localised, but that’s not always possible.
- Feature envy – occurs when you have got a method that is more interested in the details of a class other than the one that it’s in. If it seems like two methods from different classes are always “talking” to one another, maybe they should be in the same class?
- Message chains – bad message chains cause rigidity (complexity in the design). For example: a.getB().getC().doSomething(); this code is easy to break if design should be reworked/restructured. That’s a sign of the brittle design.
- Primitive obsession – this is when you rely on built-in types too much. The primitive types like int, long, float, string. For example we have some custom key or code which can be just a string, but better to avoid it and create the dedicated class, then you can even do some validation or calculation.
- Switch statements – related to the different types of object, when have to switch by type and do some action, maybe it’s better to consider polymorphism (get rid of switch at all)?
- Speculative generality – occurs when you make a superclass, interface or code that is not needed at this time, but you think that you may use it someday. In another words you over-engineering the code. In agile development, you want to be practicing just in time design. You want just enough design to take the requirements for a particular iteration to a working system.
- Refused bequest – occurs when a subclass inherit something and doesn’t need it. Say, a superclass declared a common behaviour across all its subclasses. If you find the subclasses inheriting things that they don’t use or need, then is it appropriate that the subclasses of this superclass? Maybe it will ore sense for a standalone class or maybe these unwanted behaviours should not be defined in the superclass. If only sudden subclasses are able to use them, perhaps it would be better to simply define those behaviours in those sub classes only.
Code should be Reusable, Flexible, Maintainable.
