How to Find the Stinky Parts of Your Code [Part XLI]by@mcsee

# How to Find the Stinky Parts of Your Code [Part XLI]

10mJuly 26th, 2023

## Too Long; Didn't Read

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.)

Part forty one is here!!!

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 - XLhere.

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

• 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.

• 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

• 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.

• 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

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

• 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.

• 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

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.

# 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.

• Testing

# Conclusion

Proceed with caution with tests and dates.

They are often a cause of mistakes.

# Relations

Code Smell 52 - Fragile Tests

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

L O A D I N G
. . . comments & more!