Your code smells because there are likely many instances where it could be edited or improved.

Most of these smells are just hints of something that might be wrong. Therefore, they are not required to be fixed per se… (You should look into it, though.)

Previous Code Smells

You can find all the previous code smells (Part i - XL) here.

Let's continue...

Code Smell 201 - Nested Ternaries

Arrow Code, Nested Conditions, switches, else, and many more

TL;DR: Don't use nested IFs or nested ternaries

Problems

• Readability
• Default Case

Solutions

1. Rewrite the code as an IF condition with an early return

Context

Nesting is always a problem with complexity.

We can fix it with polymorphism or early returns

Sample Code

Wrong

const getUnits = secs => (
 secs <= 60       ? 'seconds' :
 secs <= 3600     ? 'minutes' :
 secs <= 86400    ? 'hours'   :
 secs <= 2592000  ? 'days'    :
 secs <= 31536000 ? 'months'  :
                    'years' 
)

Right

const getUnits = secs => {
 if (secs <= 60) return 'seconds'; 
 if (secs <= 3_600) return 'minutes'; 
 if (secs <= 86_400) return 'hours';   
 if (secs <= 2_592_000) return 'days';    
 if (secs <= 31_536_000) return 'months';  
 return 'years' 
}

// More declarative

const getUnits = secs => {
 if (secs <= 60) return 'seconds'; 
 if (secs <= 60 * 60) return 'minutes'; 
 if (secs <= 24 * 60 * 60) return 'hours';   
 if (secs <= 30 * 24 * 60 * 60) return 'days';    
 if (secs <= 12 * 30 * 24 * 60 * 60) return 'months';  
 return 'years' 
}

Detection

• [x]Automatic

Linters can detect this complexity using parsing trees.

Tags

• IFs

Conclusion

We must deal with accidental complexity to improve code readability.

Relations

Code Smell 133 - Hardcoded IF Conditions

Code Smell 78 - Callback Hell

Code Smell 102 - Arrow Code

Disclaimer

Code Smells are my opinion.

Credits

Photo by NIKHIL on Unsplash

One of the best things to come out of the home computer revolution could be the general and widespread understanding of how severely limited logic really is.

Frank Herbert

Software Engineering Great Quotes

Code Smell 202 - God Constant Class

Constants should be together to find them easily

TL;DR: Don't define too many unrelated constants in the same class. Don't pile up the junk together.

Problems

• Bad Cohesion
• High Coupling
• Magic Numbers
• Single Responsibility principle violation

Solutions

1. Break the contents following Real World responsibilities using the MAPPER.

Context

This is a special case of a God Object restricted only to constant definitions.

The repository can be a class, file, or JSON.

Sample Code

Wrong

public static class GlobalConstants
{
    public const int MaxPlayers = 10;
    public const string DefaultLanguage = "en-US";
    public const double Pi = 3.14159; 
}

Right

public static class MaxPlayersConstants
{
    public const int MaxPlayers = 10;    
}

public static class DefaultLanguageConstants
{
    public const string DefaultLanguage = "en-US";
}

public static class MathConstants
{
    public const double Pi = 3.14159;
}

Detection

• [x]Semi-Automatic

We can tell our linters to warn us of too many constants' definitions against a preset threshold.

Tags

• Coupling

Conclusion

Finding correct responsibilities is one of our primary tasks when designing software.

Relations

Code Smell 14 - God Objects

Code Smell 29 - Settings / Configs

Code Smell 02 - Constants and Magic Numbers

More Info

Medium

Credits

Photo by Aaron Burden on Unsplash

If someone says their code was broken for a couple of days while they are refactoring, you can be pretty sure they were not refactoring.

Martin Fowler

Code Smell 203 - Irrelevant Test Information

Irrelevant data distract the reader's attention

TL;DR: Don't add unnecessary information to your assertions

Problems

• Readability
• Maintainability

Solutions

1. Remove irrelevant data
2. Leave only the needed assertions

Context

Tests should be minimal and follow the SetUp/Exercise/Assert pattern

Sample Code

Wrong

def test_formula_1_race():
    # Setup
    racers = [
        {"name": "Lewis Hamilton", "team": "Mercedes", "starting_position": 1, "car_color": "Silver", "car_model": "W12"},
        {"name": "Max Verstappen", "team": "Red Bull", "starting_position": 2, "car_color": "Red Bull", "car_model": "RB16B"},
        {"name": "Sergio Perez", "team": "Red Bull", "starting_position": 3, "car_color": "Red Bull", "car_model": "RB16B"},
        {"name": "Lando Norris", "team": "McLaren", "starting_position": 4, "car_color": "Papaya Orange", "car_model": "MCL35M"},
        {"name": "Valtteri Bottas", "team": "Mercedes", "starting_position": 5, "car_color": "Silver", "car_model": "W12"},
    ]

    # Exercise
    winner = simulate_formula_1_race(racers)

    # Test
    assert winner == "Lewis Hamilton"
    
    # This is all irrelevant to winner asserting
    assert racers[0]["car_color"] == "Silver"
    assert racers[1]["car_color"] == "Red Bull"
    assert racers[2]["car_color"] == "Red Bull"
    assert racers[3]["car_color"] == "Papaya Orange"
    assert racers[4]["car_color"] == "Silver"
    assert racers[0]["car_model"] == "W12"
    assert racers[1]["car_model"] == "RB16B"
    assert racers[2]["car_model"] == "RB16B"
    assert racers[3]["car_model"] == "MCL35M"
    assert racers[4]["car_model"] == "W12"

Right

def test_formula_1_race():
    # Setup
    racers = [
        {"name": "Lewis Hamilton", "starting_position": 1},
        {"name": "Max Verstappen", "starting_position": 2},
        {"name": "Sergio Perez", "starting_position": 3},
        {"name": "Lando Norris", "starting_position": 4},
        {"name": "Valtteri Bottas" "starting_position": 5},
    ]

    # Exercise
    winner = simulate_formula_1_race(racers)

    # Test
    assert winner == "Lewis Hamilton"  

Detection

• [x]Semi-Automatic

We can find some patterns in not needed assertions.

Tags

• Testing

Conclusion

Tests should be prose. Always focus on the reader. It might be you a couple of months from now.

Relations

Code Smell 76 - Generic Assertions

More Info

xUnit Test Patterns: Refactoring Test Code

Credits

Photo by Evan Demicoli on Unsplash

Take reasonable steps to test, document, and otherwise draw attention to the assumptions made in every module and routine.

Daniel Read

Code Smell 204 - Tests Depending on Dates

It is a good idea to assert something has happened in the future

TL;DR: Tests must be in full control and you can't manage time.

Problems

• Fragile Tests
• CI/CD Breaks

Solutions

1. Tests should be always in full environmental control.
2. Create a time source

Context

I read a Tweet about adding a fixed date to check for the removal of a feature flag (which is another code smell). The test will fail in an unpredictable way preventing releases and breaking CI/CD pipeline. There are also other bad examples we will never reach some date, tests running at midnight, different timezones, etc.

Sample Code

Wrong

class DateTest {
    @Test
    void testNoFeatureFlagsAfterFixedDate() {
        LocalDate fixedDate = LocalDate.of(2023, 4, 4);
        LocalDate currentDate = LocalDate.now();        
        Assertions.assertTrue(currentDate.isBefore(fixedDate) || !featureFlag.isOn());
    }
}

Right

class DateTest {
    @Test
    void testNoFeatureFlags() {   
        Assertions.assertFalse(featureFlag.isOn());
    }
}

Detection

• [x]Semi-Automatic

We can check assertions based on time on our tests.

Tags

• Testing

Conclusion

Proceed with caution with tests and dates.

They are often a cause of mistakes.

Relations

Code Smell 52 - Fragile Tests

More Info

Twitter

Each pattern describes a problem which occurs over and over again in our environment, and then describes the core of the solution to that problem, in such a way that you can use this solution a million times over, without ever doing it the same way twice.

Christopher Alexander

Code Smell 205 - Code in Destructors

You deallocate things in your destructors

TL;DR: Don't use destructors. And don't write functional code there.

Problems

• Coupling
• Unexpected results
• Memory leaks

Solutions

1. Don't use destructors.
2. Follow the Rule of Zero
3. Let the Garbage Collector work for you

Context

A class destructor is a special method that is called when an object is destroyed or goes out of scope.

In the past, we had no garbage collectors and destructors were responsible for cleaning up any resources that the object has acquired during its lifetime, such as closing open files or releasing memory allocated on the heap.

Nowadays, object destruction is automatic in most modern programming languages.

Sample Code

Wrong

class File {
public:
    File(const std::string& filename) {
        file_ = fopen(filename.c_str(), "r");
    }
    
    ~File() {
        if (file_) {
            fclose(file_);
        }
    }
    
private:
    FILE* file_;
};

Right

class File {
public:
    File() : file_(nullptr) {}
    
    bool Open(const std::string& filename) {
        if (file_) {
            fclose(file_);
        }
        file_ = fopen(filename.c_str(), "r");
        return (file_ != nullptr);
    }
    
    bool IsOpen() const {
        return (file_ != nullptr);
    }
    
    void Close() {
        if (file_) {
            fclose(file_);
            file_ = nullptr;
        }
    }
    
    ~File() {
        // Instead of closing the file we throw an exception 
        // If it is open (which is an invalid scenario)
        if (file_) {
            throw std::logic_error("File is still open in destructor");
        }
    }
    
private:
    FILE* file_;
};

Detection

• [x]Automatic

Linters can warn us when we write code in destructors

Exceptions

In very critical low-level code we cannot afford a garbage collector.

Exceptions are very few.

In other cases writing code in destructors is a symptom of premature optimization.

Tags

• Premature Optimization

Conclusion

Writting code in destructors is a sign of sloppiness and laziness.

We need to understand the life cycle of our objects and manage the events accurately.

Relations

Code Smell 142 - Queries in Constructors

Credits

Photo by Crawford Jolly on Unsplash

The most likely way for the world to be destroyed, most experts agree, is by accident. That's where we come in; we're computer professionals. We cause accidents.

Nathaniel S. Borenstein

5 more code smells are coming soon…

My new book about clean code is available for pre-order.

You will find several recipes like this one with a higher level of detail

https://amzn.to/44s1XdO