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
- 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
- Move your optional arguments last.
- 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
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
- Leave the code better
- 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
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
- 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
More Info
What exactly is a name - Part II Rehab
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
- 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…