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

• 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

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

1. Check the right case.
2. Assert for a functional case.
3. 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.

Tags

• Testing

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

• Test Driven Development
• The Only Design Principle
• Mutation Testing

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

1. Use a centralizing sequential stamper. (NO, not a Singleton).
2. 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.

Tags

• Bijection

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

• Timestamp proposed changes
• Build a Mapper
• What is wrong with software

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

1. Change callbacks to sequence calls.
2. Extract repeated Code
3. 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.

Tags

• Readability
• Complexity

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

1. Use intention revealing names.
2. 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....

Tags

• Readability

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

• What is in a name.
• How To Be Great At Giving Meaningful Names.

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

1. 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.

Tags

• Exceptions

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.

Relations

Code Smell 73 - Exceptions for Expected Cases

Code Smell 26 - Exceptions Polluting

More Info

• Nested Try/Catchs

Credits

Photo by David Clode on Unsplash

Thanks to @Rodrigo for his inspiration

Tweet

Writing software as if we are the only person that ever has to comprehend it is one of the biggest mistakes and false assumptions that can be made.

Karolina Szczur

And that’s all for now…

The next article will explain 5 more code smells!