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

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

September 26th, 2022

It 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. They are not required fixed per se… (You should look into it though.) The smell of code smells is a classic classic, but it's not required to be fixed. The smell is a hint that something is wrong, but you should look at it more closely if you want to improve your code smell. For example, the smell of Code Smells: "Stinky Smells"

## Code smells are a classic.

It 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. They are not required fixed per se… (You should look into it though.)

## Previous Code Smells

Let's continue...

# Code Smell 111 - Modifying Collections While Traversing

Changing a collection while traversing might lead to unexpected errors

TL;DR: Do not modify collections while traversing them

# Problems

• Unexpected Results
• Concurrency problems

# Solutions

1. Avoid altering the collections
2. Make collection copies

# Context

We over-optimize our solutions with the prejudice that copying collections are expensive.

This is not true for small and medium-size collections.

Languages iterate collections in many different ways.

Modifying them is generally not safe.

# Sample Code

## Wrong

``````Collection<Integer> people = new ArrayList<>();
// here we add elements to the collection...

for (Object person : people) {
if (condition(person)) {
people.remove(person);
}
}
// We iterate AND remove elements
``````

## Right

``````Collection<Integer> people = new ArrayList<>();
// Here we add elements to the collection...

List<Object> iterationPeople = ImmutableList.copyOf(people);

for (Object person : iterationPeople) {
if (condition(person)) {
people.remove(person);
}
}
// We iterate a copy and remove it from original

coll.removeIf(currentIndex -> currentIndex == 5);
// Or use language tools (if available)
``````

# Detection

• [x]Semi Automatic

Many languages provide control both in compile and run-time

• Fail Fast

# Conclusion

This is something we learn in our first courses.

It happens a lot in the industry and real-world software

# Relations

Code Smell 53 - Explicit Iteration

# Credits

Photo by Christine Roy on Unsplash

Bugs are bugs. You write code with bugs because you do. If it’s a safe language in the sense of run-time safe, the operating system crashes instead of doing a buffer overflow in a way that’s exploitable.

- Ken Thompson

Software Engineering Great Quotes

# Code Smell 112 - Testing Private Methods

If you work on unit testing, sooner or later you will face this dilemma

TL;DR: Don't test your private methods.

# Problems

• Breaking Encapsulation
• Code Duplication

# Solutions

1. If your method is simple, you don't need to test it.
2. If your method is complicated, you need to convert it into a Method Object.
3. Do not make your methods public for testing.
4. Do not use metaprogramming to avoid protection.
5. Do not move the private computation to helpers.
6. Do not use static methods for computations.

# Context

We test our classes and methods.

At some point, we rely on auxiliary computations and we need to test them in a white-box way.

# Sample Code

## Wrong

``````<?

final class Star {

private \$distanceInParsecs;

public function timeToReachLightToUs() {
return \$this->convertDistanceInParsecsToLightYears(\$this->distanceInParsecs);
}

private function convertDistanceInParsecsToLightYears(\$distanceInParsecs) {
return 3.26 * \$distanceInParsecs;
// function is using an argument which is already available.
// this is another smell indicator.

// We cannot test this function since it is private
}
}
``````

## Right

``````<?

final class Star {

private \$distanceInParsecs;

public function timeToReachLightToUs() {
return new ParsecsToLightYearsConverter(\$this->distanceInParsecs);
}
}

final class ParsecsToLightYearsConverter {
public function convert(\$distanceInParsecs) {
return 3.26 * \$distanceInParsecs;
}
}

final class ParsecsToLightYearsConverterTest extends TestCase {
public function testConvert0ParsecsReturns0LightYears() {
\$this->assertEquals(0, (new ParsecsToLightYearsConverter)->convert(0));
}
// we can add lots of tests and rely on this object
// So we don't need to test Star conversions.
// We can yet test Star public timeToReachLightToUs()
// This is a simplified scenario

}
``````

# Detection

• [x]Semi-Automatic

This is a semantic smell.

We can only find metaprogramming abuse on some unit frameworks.

• Test Smells

# Conclusion

With this guide, we should always choose the method object solution.

# Relations

Code Smell 21 - Anonymous Functions Abusers

Code Smell 22 - Helpers

Code Smell 18 - Static Functions

# Credits

Photo by Dan Nelson on Unsplash

Just as it is a good practice to make all fields private unless they need greater visibility, it is a good practice to make all fields final unless they need to be mutable.

- Brian Goetz

Software Engineering Great Quotes

# Code Smell 113 - Data Naming

Use entity domain names to model entity domain objects.

TL;DR: Don't name your variables as Data.

# Solutions

1. Use role suggesting names.
2. Find names in the Bijection.

# Context

We use 'data' a lot in our variables.

We are used to doing it.

Using this kind of name favors the anemic treatment of objects.

We should think about domain-specific and role-suggesting names.

# Sample Code

## Wrong

``````if (!dataExists()) {
}
``````

## Right

``````if (!peopleFound()) {
}
``````

# Detection

• [x]Semi-Automatic

We can check for this substring on our code and warn our developers.

• Naming

# Conclusion

Data is everywhere if you see the world as only data.

We can never see the data we manipulate.

We can only infer it through behavior.

We don't know the current temperature. We observe our thermometer pointing at 35 Degrees.

Our variables should reflect the domain and role they are fulfilling.

Naming them as 'data' is lazy and hinders readability.

# Relations

Code Smell 01 - Anemic Models

Code Smell 65 - Variables Named after Types

What exactly is a name - Part II Rehab

Twenty percent of all input forms filled out by people contain bad data.

- Dennis Ritchie

Software Engineering Great Quotes

# Code Smell 114 - Empty Class

Have you encountered classes without behavior? Classes are their behavior.

TL;DR: Remove all empty classes.

# Solutions

1. Remove the classes and replace them with objects instead.
2. If your classes are Anemic Exceptions, remove them.

# Context

Many developers still think classes are data repositories.

They couple different behavior concepts with returning different data.

# Sample Code

## Wrong

``````class ShopItem {
code() { }
description() { }
}

class BookItem extends ShopItem {
code() { return 'book' }
description() { return 'some book'}
}

// concrete Class has no real behavior, just return different 'data'
``````

## Right

``````class ShopItem {
constructor(code, description) {
// validate code and description
this._code = code;
this._description = description;
}
code() { return this._code }
description() { return this._description }
// dd more functions to avoid anemic classes
// getters are also code smells, so we need to iterate it
}

bookItem = new ShopItem('book', 'some book);
// create more items
``````

# Detection

• [x]Automatic

Several linters warn us of empty classes.

We can also make our own scripts using metaprogramming.

• Behavior

# Conclusion

Classes are what they do, their behavior.

Empty classes do nothing.

# Relations

Code Smell 26 - Exceptions Polluting

Code Smell 40 - DTOs

Code Smell 60 - Global Classes

Code Smell 01 - Anemic Models

# Credits

Photo by Kelly Sikkema on Unsplash

An error arises from treating object variables (instance variables) as if they were data attributes and then creating your hierarchy based on shared attributes. Always create hierarchies based on shared behaviors, side.

- David West

Software Engineering Great Quotes

# Code Smell 115 - Return True

Booleans are natural code smells. Returning and casting them is sometimes a mistake.

TL;DR: Don't return true or false. Be declarative.

# Solutions

1. Return truth value in a declarative way
2. Replace IF With polymorphism.

# Context

Dealing with low-level abstractions, we usually return booleans.

# Sample Code

## Wrong

``````boolean isEven(int num) {
if(num%2 == 0) {
return true;
} else {
return false;}
}
``````

## Right

``````boolean isEven(int numberToCheck) {
// We decouple the what (to check for even or odd)
// With how (the algorithm)
return (numberToCheck % 2 == 0);
}
``````

# Detection

• [x]Automatic

Many linters can check syntactic trees and look for explicit true/value returns.

• Primitive

# Conclusion

Search on code libraries for return true statements and try to replace them when possible.

# Relations

Code Smell 36 - Switch/case/elseif/else/if statements

Code Smell 118 - Return False

# Credits

Photo by engin akyurt on Unsplash

The good news is: Anything is possible on your computer. The bad news is: Nothing is easy.

- Ted Nelson

Software Engineering Great Quotes

And that’s all for now…

The next article will explain 5 more code smells!

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