538 reads

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

by Maximiliano ContieriApril 6th, 2022

Too Long; Didn't Read

Infinite code smells! 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.)

People Mentioned

Companies Mentioned

Coin Mentioned

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

Photo by Filipe Resmini on Unsplash

Infinite code, smells!

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 76 - Generic Assertions

Don't make weak tests to create a false sensation of coverage.

TL;DR: Test Assertions should be precise. Not too Vague and not too specific. There is no silver bullet.

Problems

False Negatives
Lack of Trust

Solutions

Check the right case.
Assert for a functional case.
Don't test implementation.

Sample Code

Wrong

square = Square(5)

assert square.area() != 0

# This will lead to false negatives since it is too vague

Right

square = Square(5)

assert square.area() = 25

# Assertion should be precise

Detection

With Mutation Testing techniques we can find these errors on our tests.

Conclusion

We should use development techniques like TDD that request concrete business cases and make concrete assertions based on our domain.

Relations

Code Smell 30 - Mocking Business

Code Smell 52 - Fragile Tests

More info

Credits

This smell was inspired by @Mario Cervera and used with his permission.

Photo by Fleur on Unsplash

A program that produces incorrect results twice as fast is infinitely slower.

John Osterhout

Code Smell 77 - Timestamps

Timestamps are widely used. They have a central issuing authority and do not go back, do they?

TL;DR: Don't use timestamps for sequence. Centralize and lock your issuer.

Problems

Bijection Fault.
Timestamp Collisions.
Timestamp precision.
Packet Disorders.
Bad Accidental Implementation (Timestamp) for an Essential Problem (Sequencing).

Solutions

Use a centralizing sequential stamper. (NO, not a Singleton).
If you need to model a sequence, model a sequence.

Sample Code

Wrong

# using time module
import time
  
# ts stores the time in seconds
ts1 = time.time()
ts2 = time.time() #might be the same!!

Right

numbers = range(1, 100000)
#create a sequence of numbers and use them with a hotspot

#or
sequence = nextNumber()

Detection

Timestamps are very popular on many languages and are ubiquitous.

We need to use them just to model... timestamps.

Conclusion

This smell was inspired by recent Ingenuity software fault.

If we don't follow our MAPPER rules and model sequences with time, we will face trouble.

Luckily, Ingenuity is a sophisticated Autonomous vehicle and has a robust fail-safe landing software.

This video describes the glitch.

https://www.youtube.com/watch?v=6IoMiwxL2wU

Relations

Code Smell 39 - new Date()

Code Smell 32 - Singletons

Code Smell 71 - Magic Floats Disguised as Decimals

More info

The most beautiful code, the most beautiful functions, and the most beautiful programs are sometimes not there at all.

Jon Bentley

Code Smell 78 - Callback Hell

Processing an algorithm as a sequence of nested callbacks is not clever.

TL;DR: Don't process calls in a callback way. Write a sequence.

Problems

Readability
Hard to debug.
Complexity

Solutions

Change callbacks to sequence calls.
Extract repeated Code
Refactor.

Sample Code

Wrong

var fs = require('fs');

var fileWithData = '/hello.world';  
fs.readFile(fileWithData, 'utf8', function(err, txt) {  
    if (err) return console.log(err);

    txt = txt + '\n' + 'Add Data!';
    fs.writeFile(fileWithData, txt, function(err) {
        if(err) return console.log(err);
        console.log('Information added');
    });
});

Right

var fs = require('fs');

function logTextWasAdded(err) {  
    if(err) return console.log(err);
    console.log('Information added');
};

function addData(error, actualText) {  
    if (error) return console.log(error);

    actualText = actualText + '\n' + 'Add data';
    fs.writeFile(fileWithData, actualText, logTextWasAdded);
}

var fileWithData = 'hello.world';  
fs.readFile(fileWithData, 'utf8', addData);

Detection

This problem shines at the naked eye. Many linters can detect this complexity and warn us.

Conclusion

Callback Hell is a very common problem in programming languages with futures or promises.

Callbacks are added in an incremental way. There's no much mess at the beginning.

Complexity without refactoring makes them hard to read and debug.

Relations

Code Smell 06 - Too Clever Programmer

Code Smell 102 - Arrow Code

There are two ways to write code: write code so simple there are obviously no bugs in it, or write code so complex that there are no obvious bugs in it.

Tony Hoare

Code Smell 79 - TheResult

If a name is already used, we can always prefix it with 'the'.

TL;DR: don't prefix your variables.

Problems

Readability
Meaningless names

Solutions

Use intention revealing names.
Avoid Indistinct noise words.

Sample Code

Wrong

var result;

result = getSomeResult();

var theResult;

theResult = getSomeResult();

Right

var averageSalary;

averageSalary = calculateAverageSalary();

//..

var averageSalaryWithRaises;

averageSalaryWithRaises = calculateAverageSalary();

Detection

As with many of our naming conventions, we can instruct our linters to forbid names like theXxx....

Conclusion

Always use intention revealing names.

If your names collide use local names, extract your methods and avoid 'the' prefixes.

Relations

Code Smell 38 - Abstract Names

More info

Credits

Photo by Josue Michel on Unsplash

One difference between a smart programmer and a professional programmer is that the professional understands that clarity is king. Professionals use their powers for good and write code that others can understand.

Robert C. Martin

Code Smell 80 - Nested Try/Catch

Exceptions are a great way of separating happy path versus trouble path. But we tend to over-complicate our solutions.

TL;DR: Don't nest Exceptions. Nobody cares of what you do in the inner blocks.

Problems

Readability

Solutions

Refactor

Sample Code

Wrong

try {
    transaction.commit();
} catch (e) {
    logerror(e);
    if (e instanceOf DBError){
      try {
          transaction.rollback();
      } catch (e) {
          doMoreLoggingRollbackFailed(e);
      }
    }
}

//Nested Try catchs
//Exception cases are 
//more important than happy path
//We use exceptions as control flow

Right

try {
    transaction.commit();
} catch (transactionError) {
    this.withTransactionErrorDo(
        transationError, transaction);
}

//transaction error policy is not defined in this function
//so we don't have repeated code
//code is more readable
//It is up to the transaction
//and the error to decide what to do

Detection

We can detect this smell using parsing trees.

Conclusion

Don't abuse exceptions, don't create Exception classes no one will ever catch, and don't be prepared for every case (unless you have a good real scenario with a covering test).

Happy path should always be more important than exception cases.