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. They are not required fixed per se… (You should look into it though.)
Previous Code Smells
- Part I
- Part II
- Part III
- Part IV
- Part V
- Part VI
- Part VII
- Part VIII
- Part IX
- Part X
- Part XI
- Part XII
- Part XIII
- Part XIV
- Part XV
- Part XVI
- Part XVII
- Part XVIII
- Part XIX
- Part XX
- Part XXI
- Part XXII
- Part XXIII
Let's continue...
Code Smell 116 - Variables Declared With 'var'
Var, Let, Const: are all the same, aren't they?
TL;DR: Choose wisely your variable names, scope, and mutability.
Problems
- Mutability
- Readability
Solutions
- Declare const all variables unless you need to change them.
Context
Most languages don't need variable declarations.
Some other languages allow us to state mutability.
We should be strict and explicit with our declarations.
Sample Code
Wrong
var pi = 3.14
var universeAgeInYears = 13.800.000.000
pi = 3.1415 // no error
universeAgeInYears = 13.800.000.001 // no error
Right
const pi = 3.14 //Value cannot mutate or change
let universeAgeInYears = 13.800.000.000 //Value can change
pi = 3.1415 // error. cannot define
universeAgeInYears = 13.800.000.001 // no error
Detection
- [x]Manual
With mutation testing, by forcing a 'const' declaration, we can check if a value remains constant and be more declarative by explicitly enforcing it.
Tags
- Mutability
- Javascript
Conclusion
Readability is always very important.
We need to explicitly state our intentions and usages.
Relations
Code Smell 86 - Mutable Const Arrays
More Info
Just as it is a good practice to make all fields private unless they need greater visibility, it is a good practice to make all fields final unless they need to be mutable.
Brian Goetz
Software Engineering Great Quotes
Code Smell 117 - Unrealistic Data
Programmers are lazy and seldom try to learn from real business domains.
TL;DR: Use real case scenarios and real data (when possible)
Problems
- Bijection Violation
- Bad test use cases
- Readability
Solutions
- Change test data for a real one.
- Use MAPPER to map real entities and real data.
Context
In the past, developers used to fake domain data.
We considered Hello Word a good practice and we tested it with abstract data.
We developed using a waterfall model very far from real users.
With bijection and MAPPER techniques, DDD and TDD, user acceptance testing became more important.
Using Agile methodologies, we need to test with real-world data.
If we find an error in a production system, we need to add a case covering the exact mistake with real data.
Sample Code
Wrong
class BookCartTestCase(unittest.TestCase):
def setUp(self):
self.cart = Cart()
def test_add_book(self):
self.cart.add_item('xxxxx', 3, 10)
#This is not a real example
self.assertEqual(self.cart.total, 30, msg='Book Cart total not correct after adding books')
self.assertEqual(self.cart.items['xxxxx'], 3, msg='Quantity of items not correct after adding book')
def test_remove_item(self):
self.cart.add_item('fgdfhhfhhh', 3, 10)
self.cart.remove_item('fgdfhhfhrhh', 2, 10)
#We made a typo since example is not a real one
self.assertEqual(self.cart.total, 10, msg='Book Cart total not correct after removing book')
self.assertEqual(self.cart.items['fgdfhhfhhh'], 1, msg='Quantity of books not correct after removing book')
Right
class BookCartTestCase(unittest.TestCase):
def setUp(self):
self.cart = Cart()
def test_add_book(self):
self.cart.add_item('Harry Potter', 3, 10)
self.assertEqual(self.cart.total, 30, msg='Book Cart total not correct after adding books')
self.assertEqual(self.cart.items['Harry Potter'], 3, msg='Quantity of items not correct after adding book')
#We don't reuse same example.
#We use a new REAL book
def test_remove_item(self):
self.cart.add_item('Divergent', 3, 10)
self.cart.remove_item('Divergent', 2, 10)
self.assertEqual(self.cart.total, 10, msg='Book Cart total not correct after removing book')
self.assertEqual(self.cart.items['Divergent'], 1, msg='Quantity of books not correct after removing book')
Detection
- [x]Manual
This is a semantic smell.
Exceptions
In some domains and under regulation, we cannot use real data.
We should fake it with meaningful data.
Tags
- Testing
Conclusion
Code comments are a code smell.
Reading tests is the only way to learn how the software behaves.
We need to be extra explicit on our tests.
Relations
Code Smell 05 - Comment Abusers
More Info
Credits
Photo by Hofmann Natalia on Unsplash
Thanks to Curtis Einsmann
You do not really understand something unless you can explain it to your grandmother.
Albert Einstein
Software Engineering Great Quotes
Code Smell 118 - Return False
Checking for a boolean condition to return a boolean value is awkward
TL;DR: Don't return explicit booleans. Most boolean usages are code smells.
Problems
- Declarativeness
- Ninja Code
- Implementative solutions
Solutions
- Return a boolean proposition instead of checking a negation.
- The answer must be a business logic formula, not an algorithm.
Context
When dealing with boolean formulas, it is more readable to show a business boolean formula than introduce a negated IF clause.
Programmers tend to return accidental implementative solutions instead of real business rules.
Sample Code
Wrong
function canWeMoveOn() {
if (work.hasPendingTasks())
return false;
else
return true;
}
Right
function canWeMoveOn() {
return !work.hasPendingTasks();
}
Detection
- [x]Automatic
Based on syntax trees, we can safely refactor the code.
Tags
- Boolean
Conclusion
Beware of returning booleans.
After the return, you will need an If statement which is also a code smell.
Relations
Code Smell 101 - Comparison Against Booleans
Code Smell 24 - Boolean Coercions
Code Smell 62 - Flag Variables
Code Smell 51 - Double Negatives
More Info
Credits
Photo by Morgan Housel on Unsplash
Thanks to Nico K. for this suggestion.
It's not at all important to get it right the first time. It's vitally important to get it right the last time.
Andrew Hunt
Software Engineering Great Quotes
Code Smell 119 - Stairs Code
Nested boolean conditions express a business rule. Not an IF
TL;DR: Avoid checking for boolean expressions and returning an explicit boolean.
Problems
- Declarativeness
- Ninja Code
- Readability
- Arrow Code
Solutions
- Return a boolean business formula value.
Context
When dealing with boolean formulas, it is more readable to show a business boolean formula than a stair of boolean checks followed by returning an explicit true/false;
Sample Code
Wrong
def is_platypus(self):
if self.is_mammal():
if self.has_fur():
if self.has_beak():
if self.has_tail():
if self.can_swim():
return True
return False
# This is also wrong since it is polluted with IFs and not readable by a biologist
def is_platypus(self):
if not self.is_mammal():
return False
if not self.has_fur():
return False
if not self.has_beak():
return False
if not self.has_tail():
return False
if not self.can_swim():
return False
return True
Right
def is_platypus(self):
return self.is_mammal() && self.has_fur() && self.has_beak() && self.has_tail() && self.can_swim()
# We can even group conditions according to animal taxonomies
Detection
- [x]Automatic
Based on syntax trees, we can safely refactor the code removing the explicit boolean value.
Tags
- Boolean
Conclusion
Beware of returning booleans.
After the return, you will need an If statement which is also a code smell.
Relations
Code Smell 101 - Comparison Against Booleans
Code Smell 24 - Boolean Coercions
Code Smell 62 - Flag Variables
Code Smell 80 - Nested Try/Catch
More Info
Credits
Photo by Jukan Tateisi on Unsplash
Thanks again to Nico K. for this suggestion.
The real hero of programming is the one who writes negative code.
Douglas McIlroy
Software Engineering Great Quotes
Code Smell 120 - Sequential IDs
Most IDS are code smells. Sequential IDs are also a vulnerability
TL;DR: Don't expose obvious consecutive IDs.
Problems
- Bijection Fault
- Security Problems
- Collisions
Solutions
- Use non-obvious keys.
- Use dark keys or UUIDs.
Context
IDs are a problem when dealing with domain objects.
IDs do not exist in the real-world so, they break our bijection.
We should only use IDs when exposing internal resources to the outer world beyond system boundaries.
These are always accidental problems and should not interfere with our models.
Sample Code
Wrong
class Book {
private Long bookId; // book knows its ID
private List<Long> authorIds; // book knows author IDs
}
Book harryPotter = new Book(1, {2});
Book cleanCode = new Book(2, {4});
Book donQuixote = new Book(3, {5});
// We can scrape from now on.
Right
class Author {
// .. Author protocol
}
class Book {
private List<Author> authors; // book knows authors
// No strange behavior. just what a book can do
// Real books don't know about IDs
// ISBN is accidental to a book. Readers don't care
}
class BookResource {
private Book resource; // The resource knows the underlying book
private id; // The id is the link we provide to external world
}
Book harryPotter = new Book(new Author('J. K. Rowling'));
Book cleanCode = new Book(new Author('Robert Martin'))
Book donQuixote = new Book(new Author('Miguel Cervantes'));
BookResource harryPotterResource = new BookResource(harryPotter, UUID.randomUUID());
// Books don't know they id. Just the resource does
Detection
- [x]Automatic
We can use Pentesting techniques against our system to detect this smell.
Tags
- Security
Conclusion
In case we need to expose internal objects to the external world, we should use non-obvious IDs.
In this way, we can detect (and block) brute force attacks monitoring the traffic and 404 errors.
More Info
Credits
Photo by Max Bender on Unsplash
Thanks @davidkroell for the KSUID tip.
The only truly secure system is one that is powered off, cast in a block of concrete and sealed in a lead-lined room with armed guards.
Gene Spafford
Software Engineering Great Quotes
And that’s all for now…
The next article will explain 5 more code smells!