Code Smells

Bloaters

Long Method

Long methods are a good indicator of a poor design and should be refactored by extracting excessive behaviours.
Ideally, a method should fit the screen without need to scroll.

Primitive Obsession

The excessive use of primitives (string, int, byte, etc.) can lead to design problems as a lot of boilerplate code, such as guard, validation, casting, parsing, etc. may be repeated ostensively to cater for its limitations. When boilerplate for these is identified, it is preferred to wrap the primitive with more feature-rich implementations. A good example are DDD Value Objects, which are created to make a more meaningful and feature-rich representation of one or more primitives.

Long Parameter List

This is the indicator of a violation of the Clean Code directive of having too many parameters. We should limit methods to 2 parameters normally, 3 for justifiable cases. Anything larger can be a code smell.

Class Doesn't Do Much

This is a bit controversial, as indicates a class offers too little in terms of functionality it becomes pointless. If the class is very small but serves a purpose, such as, a factory that knows how to create a customized objects, then it is not pointless.

Obfuscators

Regions

Regions hide code and by itself is a code smell and violates best practices.

Comments

Comments tend to become outdated when compared to the code it comments about, thus should be avoided.

Poor Names

Bad naming is an indicator of a poor code and design, thus a code smell. Always use meaningful names everywhere!

Vertical Separation

Vertical separation points to how methods are organized vertically in the class in comparison to their usage. They should be placed by use order, where the only exceptions are overloads that should be placed together always.

Object Orientation Abusers

Switch Statements

Switch statements can indicate a poor design when used to make decisions that alter behaviour. Ideally, they should be avoided or replaced with patterns. However, if indispensable, should be isolated.

Dispensables

Duplicate Code

Any DRY violation is a Duplicate Code smell. Always reuse to avoid repetition.

Dead Code

Parts of code that are not used but left over. E.g.:

public Customer GetCustomer(int id)
{
    var customer = this.service.Get(id);
    if (customer is not null)
    {
        return customer
    }
    else
    {
        return new Customer();    
    }   
    
    return null; // <- This line is never reached, thus dead code.    
} 

Couplers

Law of Demeter Violations

These violations happen when trying to access functionality not directly available to the caller. E.g. var cost = customer.Sale.Product.Price; Normally, Customer does not have direct access to a Product's price, thus accessing it in this manner violates the Law of Demeter. A Sale should provide a GetProductPrice() method that returns the price value for the customer.

References

Last modified: 03 June 2024