Spot the Smells: 7 Code Smells That Make Your Code Look Bad

1. Data Clump 🏋️♂️

When variables stick together like inseparable buddies

Ever find yourself passing the same group of variables around your methods? That's a data clump. It's like carrying groceries individually instead of using a basket—awkward and inefficient.

Why It's a Problem:

• Hidden Abstraction: Grouping related variables into their own object enhances code readability and clarity.
• Complex APIs: Methods with numerous parameters become cumbersome and harder to maintain.

How to Fix It:

• Introduce a Class: Combine these variables into a class that encapsulates their collective purpose.
• Refactor Methods: Adjust your methods to accept this new class instead of individual parameters.

Apex Example:

Smelly Code:

public void setColor(Integer red, Integer green, Integer blue) {
    // Set the color
}        

Refactored:

public class RGB {
    public Integer red;
    public Integer green;
    public Integer blue;

    public RGB(Integer red, Integer green, Integer blue) {
        this.red = red;
        this.green = green;
        this.blue = blue;
    }
}

public void setColor(RGB color) {
    // Set the color
}        

By creating the RGB class, you've bundled related data together, making your code cleaner and more intuitive.

2. Flag Argument 🚩

Methods shouldn't act like switchboards

Passing a boolean to a method to determine its behavior is like giving it a split personality; it often indicates that the method is trying to do too much.

Why It's a Problem:

• Ambiguous Behavior: A flag parameter forces a method to have multiple execution paths, making its behavior unclear.
• Violation of the Open/Closed Principle: Every time a new condition is needed, the method requires modification.
• Harder Testing: Each flag introduces at least two different execution scenarios, increasing test complexity.
• Reduced Readability: The reader must mentally parse different branches of logic instead of reading a clearly defined method.

How to Fix It:

• Extract Separate Methods: Instead of using a flag, create distinct methods for each behavior.
• Use Polymorphism: When applicable, replace flag-driven logic with subclasses or strategy patterns.
• Improve Method Naming: Clear method names will naturally convey the behavior without needing a boolean switch.

Apex Example:

Smelly Code:

public void sendNotification(Boolean isEmail) {
    if (isEmail) {
        sendEmail();
    } else {
        sendSMS();
    }
}        

Refactored:

public void sendEmailNotification() {
    sendEmail();
}

public void sendSMSNotification() {
    sendSMS();
}        

Now, each method has a clear purpose, making your code easier to read and maintain.

3. Helper Class Code Smell 🛠

When your class is just a toolbox of unrelated utils

A helper class is often created when additional methods are needed to support existing functionality, but these methods do not quite fit within the existing class. A problem can arise when these helper classes fail to convey their intended purpose, resulting in them becoming a collection of loosely related methods. Instead of enhancing code clarity, they can make it more difficult to identify where specific logic should reside.

Why It's a Problem:

• Lack of Cohesion: The class doesn't represent a single concept.
• Difficult to Maintain: As the class grows, it becomes a dumping ground for unrelated methods.

How to Fix It:

• Redistribute Methods: Move methods to classes where they logically belong.
• Create New Classes: If the methods don't fit anywhere, create new classes that encapsulate the related functionality.

Apex Example:

Smelly Code:

public class Utils {
    public static Integer add(Integer firstNumber, Integer secondNumber) {
        return firstNumber + secondNumber;
    }

    public static Integer subtract(Integer firstNumber, Integer secondNumber) {
        return firstNumber - secondNumber;
    }

    public void sendSMSNotification() {
        sendSMS();
    }
}        

Refactored:

public class Calculator {
    public Integer add(Integer firstNumber, Integer secondNumber) {
        return firstNumber + secondNumber;
    }

    public Integer subtract(Integer firstNumber, Integer secondNumber) {
        return firstNumber - secondNumber;
    }
}

public class Notificator {
    public void sendSMSNotification() {
        sendSMS();
    }
}        

By creating Calculator and Notificator classes, you've given these methods a proper home, enhancing code organization.

4. Long Parameter List 📜

When your method signatures resemble lengthy shopping lists

Have you ever tried to juggle too many items at once? That's similar to handling methods with too many parameters. It not only makes the method difficult to comprehend, but it also heightens the likelihood of making mistakes.

Why It's a Problem:

• Hard to Use: Methods with numerous parameters require more effort to understand and utilize correctly.
• Increased Complexity: A high number of parameters can lead to inconsistent inputs, making the method's behavior unpredictable.
• Single Responsibility Principle Violation: Too many parameters often indicate that a method is trying to accomplish too much at once.

How to Fix It:

• Introduce Parameter Object: Group related parameters into a single object to simplify method signatures.
• Preserve the Whole Object: Instead of passing individual attributes, pass the entire object.
• Replace Parameter with Query: Have the method obtain the data it needs internally rather than relying on external parameters.

Apex Example:

Smelly Code:

public void registerUser(String firstName, String lastName, String email, String phone, String address) {
    // Registration logic
}        

Refactored:

public class User {
    public String firstName;
    public String lastName;
    public String email;
    public String phone;
    public String address;

    // Constructor
    public User(String firstName, String lastName, String email, String phone, String address) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.email = email;
        this.phone = phone;
        this.address = address;
    }
}

public void registerUser(User user) {
    // Registration logic
}        

By encapsulating user details into a User class, the registerUser method becomes cleaner and more manageable.

5. Magic Number 🎩✨

Numbers in code should be meaningful, not mysterious.

A magic number is a hardcoded numerical value without explanation, which makes the code difficult to understand and modify. These numbers are dispersed throughout the codebase, and their purpose is not immediately clear.

Why It's a Problem:

• Lack of Meaning: A random number in the code provides no context, making it difficult to understand its purpose.
• Difficult Maintenance: Changing a magic number requires searching through the codebase, increasing the risk of mistakes.
• Inconsistent Usage: Magic numbers can appear in multiple places, leading to inconsistencies when updates are needed.
• Reduced Readability: Future developers (or even you) will struggle to determine what the number represents.

How to Fix It:

• Use Named Constants: Replace magic numbers with well-named constants that describe their purpose.
• Encapsulate Values: If the number is part of a calculation, move it into a configuration file or settings class.
• Improve Documentation: If a number is unavoidable, provide a clear comment explaining its significance.

Apex Example:

Smelly Code:

Decimal discount = totalAmount * 0.15;        

Refactored:

public static final Decimal DISCOUNT_RATE = 0.15;
Decimal discount = totalAmount * DISCOUNT_RATE;        

By defining DISCOUNT_RATE, the purpose of 0.15 is clear, reducing confusion and improving maintainability.

6. Status Variable 🚥

Status Variables: Unnecessary Complexity

A status variable tracks state changes within a method. This often indicates that the method is taking on too many responsibilities, resulting in unclear and harder-to-maintain code.

Why It's a Problem:

• Code Complexity: Methods using status variables tend to have multiple execution paths, making them harder to understand.
• Higher Risk of Bugs: If a status variable is not correctly updated, the method’s logic may become faulty.
• Violates Single Responsibility Principle: A method tracking multiple states may be responsible for more than one concern.

How to Fix It:

• Refactor into Separate Methods: Break down methods to remove the need for status tracking.
• Use Guard Clauses: Instead of relying on status variables, return early when conditions are met.
• Leverage Object-Oriented Principles: Encapsulate state-related logic within a dedicated class or object.

Apex Example:

Smelly Code:

public Integer findFooIndex(List<String> names) {
    Boolean found = false;
    Integer i = 0;
    
    while (!found && i < names.size()) {
        if (names[i] == 'foo') {
            found = true;
        } else {
            i++;
        }
    }
    return i;
}        

Refactored:

public Integer findFooIndex(List<String> names) {
    for (Integer i = 0; i < names.size(); i++) {
        if (names[i] == 'foo') {
            return i;
        }
    }
    return -1;
}        

7. "What" Comment 📝

Comments should explain 'why,' not 'what.'

Comments that explain what the code does often suggest that the code isn't self-explanatory. Instead of relying on comments for clarity, aim to enhance the readability of the code itself.

Why It's a Problem:

• Redundant Information: If the code already shows what it does, a comment repeating the same thing is unnecessary.
• Can Become Outdated: If the code changes but the comment isn't updated, it can mislead developers.
• Masks Poor Code: Instead of fixing unclear code, developers might add comments to explain it, leading to technical debt.

How to Fix It:

• Use meaningful methods and variable names: Instead of adding comments, make your code self-documenting.
• Break down complex logic into smaller functions: If a piece of logic is too complex, extract it into a method with a descriptive name.
• Only add comments where necessary: Use comments to explain why something is done, not what is being done.

Apex Example:

Smelly Code:

public class ReportGenerator {
    public void runReport() {
        // Creating Report
        Report report = generateReport();
        // Sending Report
        sendReport(report);
    }
}        

Refactored:

public class ReportGenerator {
    public void executeReport() {
        Report report = createReport();
        dispatchReport(report);
    }

    private Report createReport() {
        // Logic to generate the report
    }

    private void dispatchReport(Report report) {
        // Logic to send the report
    }
}        

By renaming methods and breaking down tasks into well-named functions, the code becomes self-explanatory, eliminating the need for 'what' comments.

Final Thoughts

Identifying and removing code smells is crucial for writing clean and maintainable Apex code. By tackling these common issues, you enhance the readability, debuggability, and extensibility of your codebase. Refactoring doesn't have to be a daunting task; instead, small, incremental improvements can lead to significant benefits over time. Be vigilant for these code smells, and both your future self and your teammates will appreciate your efforts! 🚀