Is Your Code Holding You Back? Let’s Refactor it Together ??

We all know that code can become messy, confusing, and hard to maintain over time. I have realized over last 8 years that only "Code Smells" can help use identify the potential problems.

In this article , let's explore common code smells and how to address them through refactoring. For ease of understanding, I have used c# language. The best part is these techniques are language independent.

Identifying the Code Smells

Before we dive into refactoring techniques, I feel it would be nice to understand some code smells which are really common.

1. Oddball solutions:

When the same problem is solved in multiple ways across the entire codebase. This can lead to inconsistency. This can lead to understanding problem for the people who tries to understand the code.

Scenario: User authentication

Imagine we are working with an application that handles user authentication. Over time, different developers have implemented user authentication in various parts of the codebase unknowingly. I see this as a problem of inconsistent methods for performing the same task. Yes, this results in an "oddball solution."

Initial implementation with oddball ways

In this scenario, let’s say there are two different ways to authenticate a user, implemented by different developers.

Method 1

public class UserService
{
    public bool AuthenticateUser(string username, string password)
    {
        // performing authentication using a basic comparison
        var user = GetUserFromDatabase(username);
        return user != null && user.Password == password;
    }

    private User GetUserFromDatabase(string username)
    {
        // just simulating database access
        return new User { Username = username, Password = "securepassword" };
    }
}
        

Method 2

public class LoginController
{
    public bool ValidateCredentials(string username, string password)
    {
        // performing authentication using a hashed password comparison
        var user = FetchUser(username);
        return user != null && VerifyPassword(password, user.HashedPassword);
    }

    private User FetchUser(string username)
    {
        // Simulate database access
        return new User { Username = username, HashedPassword = "hashedpassword" ;     
    }

    private bool VerifyPassword(string enteredPassword, string storedHashedPassword)
    {
        // simulating password hash verification
        return HashPassword(enteredPassword) == storedHashedPassword;
    }

    private string HashPassword(string password)
    {
        // simulating hashing (not a real hash function of course)
        return "hashed" + password;
    }
}
        

I see couple of problems with the oddball solutions here

• inconsistency: There are two different ways of authenticating users (AuthenticateUser and ValidateCredentials). One uses plain text comparison, and the other uses hashed passwords.
• maintenance nightmare: If the authentication logic changes (e.g., implementing multi-factor authentication), both methods need to be updated.
• potential security risks: The plain text comparison in AuthenticateUser is less secure and inconsistent with modern security practices.

Let's refactor the code:

public class UserService
{
    public bool AuthenticateUser(string username, string password)
    {
        // fetch user and authenticate using a single consistent method
        var user = FetchUserFromDatabase(username);
        return user != null && VerifyPassword(password, user.HashedPassword);
    }

    private User FetchUserFromDatabase(string username)
    {
        // simulating the database access
        return new User { Username = username, HashedPassword = "hashedsecurepassword" };
    }

    private bool VerifyPassword(string enteredPassword, string storedHashedPassword)
    {
        return HashPassword(enteredPassword) == storedHashedPassword;
    }

    private string HashPassword(string password)
    {
        return "hashed" + password;
    }
}

public class LoginController
{
    private readonly UserService _userService;

    public LoginController(UserService userService)
    {
        _userService = userService;
    }

    public bool ValidateCredentials(string username, string password)
    {
        // just delegate to the UserService for consistency
        return _userService.AuthenticateUser(username, password);
    }
}
        

What benefits did we get?

1. consistency: The AuthenticateUser method is now the single source of truth for user authentication. And it is used throughout the application.
2. security: All user authentication now relies on hashed password verification. Isn't it more secure than plain text comparison?
3. easier maintenance: Future changes to the authentication logic only need to be made in one place (UserService). This reduces the risk of errors and simplifying updates.

Now, let’s dive into two important refactoring techniques in object-oriented programming:

Using creation methods instead of constructors and Constructor chaining to minimize redundancy.

Both of the techniques help us improving the clarity, maintainability, and flexibility of our code of course.

1. Use creation methods instead of constructors

Creation methods are static methods basically, so they act as factory methods to create instances of a class. They are used instead of constructors to provide more meaningful and descriptive ways to create objects. This technique is particularly useful when a class has multiple constructors with similar parameter lists, which can be confusing and error-prone.

But why use creation methods?

• readability: Creation methods can have descriptive names that clearly indicate the intent of object creation, making the code easier to read and understand.
• flexibility: You can have multiple creation methods with different names and signatures, providing flexibility in how objects are created.
• encapsulation: You can keep the constructor private, forcing the use of creation methods and allowing control over how instances are created.
• avoid overloaded constructors: Overloading constructors with similar signatures can lead to confusion and errors. Creation methods clarify which parameters are used in what context.

Example: without creation methods

Suppose we have a User class with multiple constructors:

public class User
{
    public string Name { get; }
    public int Age { get; }
    public bool IsAdmin { get; }

    // constructor for a regular user
    public User(string name, int age)
    {
        Name = name;
        Age = age;
        IsAdmin = false;
    }

    // constructor for an admin user
    public User(string name, int age, bool isAdmin)
    {
        Name = name;
        Age = age;
        IsAdmin = isAdmin;
    }
}
        

In this example, it's not immediately clear what true or false in the isAdmin parameter represents when calling the constructor. It also allows potential misuse or errors.

Example: with creation methods

Now, let’s refactor this to use creation methods:

public class User
{
    public string Name { get; }
    public int Age { get; }
    public bool IsAdmin { get; }

    // private constructor
    private User(string name, int age, bool isAdmin)
    {
        Name = name;
        Age = age;
        IsAdmin = isAdmin;
    }

    // creation method for a regular user
    public static User CreateRegularUser(string name, int age)
    {
        return new User(name, age, false);
    }

    // creation method for an admin user
    public static User CreateAdminUser(string name, int age)
    {
        return new User(name, age, true);
    }
}
        

Benefits of using creation methods:

1. descriptive and clear: Methods like CreateRegularUser and CreateAdminUser clearly describe what kind of user is being created. So, this way it improves code readability.
2. reduced risk of errors: By hiding the constructor and using creation methods, we reduce the chance of someone accidentally creating a user with the wrong isAdmin flag.
3. centralized object creation logic: We can easily add logic to the creation methods if needed (e.g., logging, validation), and it will be applied consistently across all instances.

2. Constructor chaining to minimize redundancy

Constructor chaining is a technique where one constructor calls another constructor within the same class to avoid duplicating code. This is especially useful when you have multiple constructors that share common initialization logic.

Why use constructor chaining?

• avoid code duplication: Shared initialization code only needs to be written once.
• centralized initialization logic: If we need to change the initialization logic, we only have to update it in one place.
• cleaner code: Reduces clutter and makes the constructors easier to maintain and understand in long run.

Example: without constructor chaining

Consider a Book class that has several constructors:

public class Book
{
    public string Title { get; }
    public string Author { get; }
    public decimal Price { get; }

    // constructor for a book with a title and author
    public Book(string title, string author)
    {
        Title = title;
        Author = author;
        Price = 0; // Default price
    }

    // constructor for a book with a title, author, and price
    public Book(string title, string author, decimal price)
    {
        Title = title;
        Author = author;
        Price = price;
    }
}
        

So, if we need to change the way the Price is initialized (say, to a different default value), we’d have to update multiple constructors.

Example: with constructor chaining

let’s refactor this to use constructor chaining:

public class Book
{
    public string Title { get; }
    public string Author { get; }
    public decimal Price { get; }

    // our primary constructor
    public Book(string title, string author, decimal price)
    {
        Title = title;
        Author = author;
        Price = price;
    }

    // chained constructor with a default price
    public Book(string title, string author)
        : this(title, author, 0m) // chaining to the primary constructor
    {
    }
}
        

Benefits of constructor chaining:

1. single point of initialization: We saw that the primary constructor handles the initialization logic and thus ensure consistency across all object instances.
2. reduced maintenance: If the logic for initializing a book needs to change, we only update the primary constructor, not all overloaded versions.
3. cleaner and more maintainable code: The constructors are less cluttered, and the code is easier to read and maintain.

Conclusion

Both creation methods and constructor chaining are valuable techniques in object-oriented programming:

• Creation Methods helps us enhancing readability and flexibility by providing descriptive, intention revealing names for object creation. They also kind of encapsulate the construction process, which can prevent errors and enforce consistent object creation practices.
• Constructor Chaining minimizes redundancy by centralizing initialization logic, making the code cleaner, easier to maintain, and reducing the likelihood of errors due to duplicated code. Thus making our life easier.

Together, these techniques help create a more maintainable, understandable, and flexible codebase, making it easier to manage both now and in the future.