691 reads

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

by Maximiliano ContieriOctober 19th, 2022

Too Long; Didn't Read

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.) The smell of code smells is a classic. It smells because of many instances of things that could be wrong. The smell is not required to be fixed, but you should look at it more closely. It's a classic classic. Code smells are a classic, but there are many ways to improve your code smell.

People Mentioned

Company Mentioned

featured image - How to Find the Stinky Parts of Your Code [Part XXIV]

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

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.

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.

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.

Conclusion

Beware of returning booleans.

After the return, you will need an If statement which is also a code smell.

Relations

Code Smell 115 - Return True

Code Smell 101 - Comparison Against Booleans

Code Smell 24 - Boolean Coercions

Code Smell 62 - Flag Variables

Code Smell 102 - Arrow Code

Code Smell 51 - Double Negatives

More Info

How to Get Rid of Annoying Ifs Forever

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.

Conclusion

Beware of returning booleans.

After the return, you will need an If statement which is also a code smell.

Relations

Code Smell 115 - Return True

Code Smell 118 - Return False

Code Smell 101 - Comparison Against Booleans

Code Smell 24 - Boolean Coercions

Code Smell 62 - Flag Variables

Code Smell 102 - Arrow Code

Code Smell 80 - Nested Try/Catch

More Info

How to Get Rid of Annoying Ifs Forever

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