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 Remove getter comments 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 Semi-Automatic [x] 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 Break your class 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 Automatic [x] 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 on Marcin Simonides 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 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 Automatic [x] 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 on Eden Constantino 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 Remove nulls Deal with undefined Context , Optionals, Coalescence, and many other solutions help us deal with the infamous nulls. Optional Chaining 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 Automatic [x] 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. , Truthy and Falsy are also code smells. Nullish Values We need to aim higher and make cleaner code. : remove all nulls from your code. The good : use optional chaining. The bad : not treating nulls at all. The ugly 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 on engin akyurt 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 violation Anthropomorphism Solutions 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 Semi-Automatic [x] 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 on Piret Ilver 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.

ChatGPT: What It's Teaching to a +25 Year Senior Programmer

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

Buy My Book

Share your ideas with me!

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

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

10 Predictions About the Future of Finance

MS Marks Significant Milestone as ADGM-Licensed Company Service Provider

Three Numbers. That’s All Your AI Needs to Work

Code Smell 318 - Wasting Time Refactoring Dirty Code

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

10 Predictions About the Future of Finance

MS Marks Significant Milestone as ADGM-Licensed Company Service Provider

Three Numbers. That’s All Your AI Needs to Work

Code Smell 318 - Wasting Time Refactoring Dirty Code

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

Light-Mode

Classic

Newspaper

Minty

Dark-Mode

Neon Noir

Minty

HN StartUps