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

Let's continue...

Code Smell 146 - Getter Comments

Comments are a code Smell. Getters are another code smell. Guess what?

TL;DR: Don't use getters. Don't comment getters

Problems

• Comment Abusers
• Readability
• Getters

Solutions

1. Remove getter comments
2. Remove getters

Context

A few decades ago, we used to comment on every method. Even trivial ones.

Comment should describe only a critical design decision.

Sample Code

Wrong

pragma solidity >=0.5.0 <0.9.0;

contract Property {
    int private price;   

    function getPrice() public view returns(int) {           
           /* returns the Price  */

        return price;
    }
}

Right

pragma solidity >=0.5.0 <0.9.0;

contract Property {
    int private _price;   

    function price() public view returns(int) {        
        return _price;
    }
}

Detection

• [x]Semi-Automatic

We can detect if a method is a getter and has a comment.

Exceptions

The function needs a comment, that is accidentally a getter and the comment is related to a design decision

Tags

• Comments

Conclusion

Don't comment getters.

They add no real value and bloat your code.

Relations

Code Smell 05 - Comment Abusers

Code Smell 68 - Getters

Code Smell 01 - Anemic Models

Credits

Photo by Reimond de Zuñiga on Unsplash

Code should be remarkably expressive to avoid most of the comments. There'll be a few exceptions, but we should see comments as a 'failure of expression' until proven wrong.

Robert Martin

Software Engineering Great Quotes

Code Smell 147 - Too Many Methods

Util classes are great to gather protocol.

TL;DR: Don't add accidental protocol to your classes

Problems

• Readability
• Single Responsibility Violation
• Bad Cohesion
• High Coupling
• Low Reusability

Solutions

1. Break your class
2. Extract Class

Refactorings

Refactoring 007 - Extract Class

Context

We tend to put a protocol in the first class we find.

That's not a problem.

We just need to refactor.

Sample Code

Wrong

public class MyHelperClass {
  public void print() { }
  public void format() { }
  // ... many methods more

  // ... even more methods 
  public void persist() { }
  public void solveFermiParadox() { }      
}

Right

public class Printer {
  public void print() { }
}

public class DateToStringFormatter {
  public void format() { }
}

public class Database {
  public void persist() { }
}

public class RadioTelescope {
  public void solveFermiParadox() { }
}   

Detection

• [x]Automatic

Most linters count methods and warn us.

Relations

Code Smell 124 - Divergent Change

Code Smell 143 - Data Clumps

Code Smell 94 - Too Many imports

Code Smell 22 - Helpers

Code Smell 34 - Too Many Attributes

More Info

Refactoring Guru

Tags

• Cohesion
• Bloaters

Conclusion

Splitting classes and protocol is a good practice to favor small and reusable objects.

Credits

Photo by Marcin Simonides on Unsplash

There is no code so big, twisted, or complex that maintenance can't make it worse.

Gerald M. Weinberg

Code Smell 148 - ToDos

We buy debt for our future selves. It is payback time.

TL;DR: Don't leave TODOs in your code. Fix them!

Problems

• Technical Debt
• Readability
• Lack of Confidence

Solutions

1. Fix your TODOs

Context

We encounter TODOs in our code. We count them.

We seldom address it.

We started owing the technical debt.

Then we pay the debt + the interest.

A few months after, we pay more interest than the original debt.

Sample Code

Wrong

public class Door
{ 
    private Boolean isOpened;
    
    public Door(boolean isOpened)
    {       
        this.isOpened = isOpened;
    }      
    
    public void openDoor()
    {
        this.isOpened = true;
    }
    
    public void closeDoor()
    {
        // TODO: Implement close door and cover it
    }      
    
}

Right

public class Door
{
 
    private Boolean isOpened;
    
    public Door(boolean isOpened)
    {       
        this.isOpened = isOpened;
    }      
    
    public void openDoor()
    {
        this.isOpened = true;
    }
    
    public void closeDoor()
    {
        this.isOpened = false;
    }      
    
}

Detection

• [x]Automatic

We can count TODOs.

Tags

• Technical Debt

Conclusion

We can count TODOs.

Most linters do it.

We need the policy to reduce them.

If we are using TDD, we write the missing code right away.

In this context, TODOs are only valid when doing Depth First development to remember open paths to visit.

More Info

• Should you put ToDos?
• Broken Windows Theory

Credits

Photo by Eden Constantino on Unsplash

After you finish the first 90% of a project, you have to finish the other 90%.

Michael Abrash

Code Smell 149 - Optional Chaining

Our code is more robust and legible. But we hide NULL under the rug.

TL;DR: Avoid Nulls and undefined. If you avoid them you will never need Optionals.

Problems

• Nulls
• IF Polluting

Solutions

1. Remove nulls
2. Deal with undefined

Context

Optional Chaining, Optionals, Coalescence, and many other solutions help us deal with the infamous nulls.

There's no need to use them once our code is mature, robust, and without nulls.

Sample Code

Wrong

const user = {
  name: 'Hacker'
};

if (user?.credentials?.notExpired) {
  user.login();
}

user.functionDefinedOrNot?.();

// Seems compact but it is hacky and has lots
// of potential NULLs and Undefined

Right

function login() {}

const user = {
  name: 'Hacker',
  credentials: { expired: false }
};

if (!user.credentials.expired) {
  login();
}

// Also compact 
// User is a real user or a polymorphic NullUser
// Credentials are always defined.
// Can be an instance of InvalidCredentials
// Assuming we eliminated nulls from our code

if (user.functionDefinedOrNot !== undefined) {  
    functionDefinedOrNot();
}

// This is also wrong.
// Explicit undefined checks are yet another code smell

Detection

• [x]Automatic

This is a Language Feature.

We can detect it and remove it.

Tags

• Null

Conclusion

Many developers feel safe polluting the code with null dealing.

In fact, this is safer than not treating NULLs at all.

Nullish Values, Truthy and Falsy are also code smells.

We need to aim higher and make cleaner code.

The good: remove all nulls from your code.

The bad: use optional chaining.

The ugly: not treating nulls at all.

Relations

Code Smell 145 - Short Circuit Hack

Code Smell 12 - Null

Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)

More Info

• Optional Chaining Reference

Null: The Billion Dollar Mistake

How to Get Rid of Annoying IFs Forever

WAT?

Credits

Photo by engin akyurt on Unsplash

He who fights with monsters might take care lest he thereby become a monster. And if you gaze for long into an abyss, the abyss gazes also into you.

Nietzsche

Code Smell 150 - Equal Comparison

Every developer compares attributes equally. They are mistaken.

TL;DR: Don't export and compare, just compare.

Problems

• Encapsulation break
• Code Duplication
• Information Hiding violation
• Anthropomorphism violation

Solutions

1. Hide the comparison in a single method

Context

Attribute comparison is heavily used in our code.

We need to focus on behavior and responsibilities.

It is an object's responsibility to compare with other objects. Not our own.

Premature Optimizers will tell us this is less performant.

We should ask them for real evidence and contrast the more maintainable solution.

Sample Code

Wrong

if (address.street == 'Broad Street') {
  

if (location.street == 'Bourbon St') {
  
// 15000 usages in a big system  
// Comparisons are case sensitive

Right

if (address.isAtStreet('Broad Street') {
    }

// ...

if (location.isAtStreet('Bourbon St') {
    }  
// 15000 usages in a big system  
  
function isAtStreet(street) {
  // We can change Comparisons to 
  // case sensitive in just one place. 
}

Detection

• [x]Semi-Automatic

We can detect attribute comparisons using syntax trees.

There can be good uses for primitive types as with many other smells.

Tags

• Encapsulation

Conclusion

We need to put responsibilities in a single place.

Comparing is one of them.

If some of our business rules change, we need to change a single point.

Relations

Code Smell 63 - Feature Envy

Code Smell 101 - Comparison Against Booleans

Code Smell 122 - Primitive Obsession

Credits

Photo by Piret Ilver on Unsplash

Behavior is the most important thing about software. It is what users depend on. Users like it when we add behavior (provided it is what they really wanted), but if we change or remove behavior they depend on (introduce bugs), they stop trusting us.

Michael Feathers

Next article: 5 more code smells.