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

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

Let's continue...

Code Smell 171 - Plural Classes

Classes are “my precious”

TL;DR: Classes represent concepts. And concepts are singular.

Problems

• Naming
• Code Standards

Solutions

1. Rename classes to singular

Context

Naming things is hard.

We need to agree on certain rules.

Sample Code

Wrong

class Users

Right

class User

Detection

• [x]Automatic

This is a syntactic rule.

Tags

• Naming

Conclusion

Name concepts in the singular.

Classes are concepts.

More Info

What exactly is a name - Part II Rehab

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Anton Malanin on Unsplash

We are still in the infancy of naming what is really happening on software development projects.

Alistair Cockburn

Software Engineering Great Quotes

Code Smell 172 - Default Argument Values Not Last

Function signature should be error prune

TL;DR: Don't use Optional Arguments before mandatory ones. In fact: Don't use Optional Arguments at all

Problems

• Fail Fast principle violation
• Readability

Solutions

1. Move your optional arguments last.
2. Avoid Optional Arguments.

Context

Optional Arguments are a code smell.

Defining optional arguments before mandatory ones is an error.

Sample Code

Wrong

<?

function buildCar($color = "red", $model) {
  //...
}  
// First argument with optional argument

buildCar("Volvo");  
// Runtime error: Too few arguments to function buildCar()

Right

<?

function buildCar($model, $color = "Red", ){...}

buildCar("Volvo")}} 
// Works as expected

def functionWithLastOptional(a, b, c='foo'):
    print(a)
    print(b)
    print(c)
functionWithLastOptional(1, 2)

def functionWithMiddleOptional(a, b='foo', c):
    print(a)
    print(b)
    print(c)
functionWithMiddleOptional(1, 2)

# SyntaxError: non-default argument follows default argument

Detection

• [x]Automatic

Many Linters can enforce this rule since we can derive it from function signature.

Also, many compilers directly forbid it.

Tags

• Readability

Conclusion

Try to be strict when defining functions to avoid coupling.

Relations

Code Smell 19 - Optional Arguments

More Info

Fail Fast

Rule on Sonar Source

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Manuel Torres Garcia on Unsplash

Programs are meant to be read by humans and only incidentally for computers to execute.

Donald Knuth

Code Smell 173 - Broken Windows

Always leave the campground cleaner than you found it.” If you find a mess on the ground, you clean it up regardless of who might have made it.

TL;DR: Follow Uncle Bob's boy scout rule.

Problems

• Readability
• Maintainability

Solutions

1. Leave the code better
2. Change it

Context

We read code many more times than we write.

We must take ownership of code with errors and leave it better.

Sample Code

Wrong

    int mult(int a,int  other) 
    { int prod
      prod= 0; 
      for(int i=0;i<other  ;i++) 
        prod+= a ; 
         return prod; 
    } 

// Formatting, naming, assignment and standards inconsistent

Right

int multiply(int firstMultiplier, int secondMultiplier) {
  int product = 0; 
  for(int currentIndex=0; currentIndex<secondMultiplier; currentIndex++) {
    product += firstMultiplier; 
  }
  return product; 
} 

// or just multiply them :)

Detection

• [x]Semi-Automatic

We can use other code smell detectors and leave the code in a better shape.

Tags

• Standards

Conclusion

We must follow the Boy Scout rule and leave the code better.

Relations

Code Smell 164 - Mixed Indentations

CoDe SmElL 159 - mIxEd_cASe

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Pawel Czerwinski on Unsplash

One broken window, left unrepaired, instills in the inhabitants of the building a sense of abandonment. People start littering. Graffiti appears. Serious structural damage begins. In a relatively short span of time, the building becomes damaged

Andy Hunt

Code Smell 174 - Class Name in Attributes

Redundancy in names is a bad smell. Names should be contextual

TL;DR: Don't prefix your attributes with your class name

Problems

• Not Contextual Names

Solutions

1. Remove the class prefix from the attribute

Context

This is a naming smell, we should not read attributes in isolation and names are contextual.

Sample Code

Wrong

public class Employee {

   String empName = "John";

   int empId = 5;

   int empAge = 32;

}

Right

public class Employee {

   String name;

   int id; // Ids are another smell

   int age; // Storing the age is yet another smell

}

Detection

• [x]Semi-Automatic

When the full name is included in the prefix, our linters can warn us.

Tags

• Naming

Conclusion

Careful naming is a very important task.

We need to name after the behavior, not type or data

Relations

Code Smell 188 - Redundant Parameter Names

Code Smell 141 - IEngine , AVehicle, ImplCar

Code Smell 96 - My Objects

More Info

What exactly is a name - Part II Rehab

• Linux Hint

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Phoenix Han on Unsplash

Copying skips understanding. Understanding is how you grow. You have to understand why something works or why something is how it is. When you copy it, you miss that. You just repurpose the last layer instead of understanding all the layers underneath.

Jason Fried

Code Smell 175 - Changes Without Coverage

If your merge request has no test changed, you haven't finished your job

TL;DR: Don't change the code without breaking some tests.

Problems

• Quality
• Maintainability

Solutions

1. Cover your code.

Context

When you need to make a change, you need to update the live specification of your code.

That's what tests are for.

Instead of generating dead documentation of what your code does, you should write a covering use scenario.

If you change uncovered tests, you need to add coverage.

Suppose you change the code with existing coverage. Lucky you! Go and change your broken tests.

Sample Code

Wrong

export function sayHello(name: string): string {

  const lengthOfName = name.length;

-  const salutation = `How are you ${name}?, I see your name has ${lengthOfName} letters!`;

+  const salutation = `Hello ${name}, I see your name has ${lengthOfName} letters!`;

  return salutation;

} 

Right

export function sayHello(name: string): string {

  const lengthOfName = name.length;

-  const salutation = `How are you ${name}?, I see your name has ${lengthOfName} letters!`;

+  const salutation = `Hello ${name}, I see your name has ${lengthOfName} letters!`;

  return salutation;

}



import { sayHello } from './hello';



test('given a name produces the expected greeting', () => {

  expect(sayHello('Alice')).toBe(

    'Hello Alice, I see your name has 6 letters!'

  );

});

Detection

• [x]Automatic

We can ensure all our merge requests include test code.

Exceptions

If your code and your tests harness live in different repositories, you might have different pull requests.

Tags

• Quality

Conclusion

Test coverage is as important as functional code.

The test system is our first and more loyal customer.

We need to care for them.

Relations

Code Smell 05 - Comment Abusers

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Vincent Péré on Unsplash

Before you use a method in a legacy system, check to see if there are tests for it. If there aren’t, write them. When you do this consistently, you use tests as a medium of communication.

Michael Feathers

5 more code smells are coming soon…