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
- [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
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
- [x]Automatic
Most linters count methods and warn us.
Relations
Code Smell 124 - Divergent Change
Code Smell 94 - Too Many imports
Code Smell 34 - Too Many Attributes
More Info
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
- 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
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
- Remove nulls
- 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 69 - Big Bang (JavaScript Ridiculous Castings)
More Info
Null: The Billion Dollar Mistake
How to Get Rid of Annoying IFs Forever
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
- 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 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.