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
Let's continue...
Code Smell 106 - Production Dependent Code
Don't add IFs checking for the production environment.
TL;DR: Avoid adding conditionals related to production
Problems
- Fail fast principle violation
- Lack of testability
Solutions
- If completely necessary, model environments and test ALL of them.
Context
Sometimes, We need to create different behaviors in development and production.
For example the strength of the passwords.
In this case, we need to configure the environment with the strength strategy and test the strategy and not the environment itself.
Sample Code
Wrong
def send_welcome_email(email_address, environment):
if ENVIRONMENT_NAME == "production":
print(f"Sending welcome email to {email_address} from Bob Builder <bob@builder.com>")
else:
print("Emails are sent only on production")
send_welcome_email("john@doe.com", "development")
# Emails are sent only on production
send_welcome_email("john@doe.com", "production")
# Sending welcome email to john@doe.com from Bob Builder <bob@builder.com>
Right
class ProductionEnvironment:
FROM_EMAIL = "Bob Builder <bob@builder.com>"
class DevelopmentEnvironment:
FROM_EMAIL = "Bob Builder Development <bob@builder.com>"
# We can unit test environments
# and even implement different sending mechanisms
def send_welcome_email(email_address, environment):
print(f"Sending welcome email to {email_address} from {environment.FROM_EMAIL}")
# We can delegate into a fake sender (and possible logger)
# and unit test it
send_welcome_email("john@doe.com", DevelopmentEnvironment())
# Sending welcome email to john@doe.com from Bob Builder Development <bob@builder.com>
send_welcome_email("john@doe.com", ProductionEnvironment())
# Sending welcome email to john@doe.com from Bob Builder <bob@builder.com>
Detection
- [x]Manual
This is a design smell.
We need to create empty development/production configurations and delegate them with customizable polymorphic objects.
Tags
- Coupling
Conclusion
Avoid adding untestable conditionals.
Create configurations delegating business rules.
Use abstractions, protocol, and interfaces, avoid hard hierarchies.
Relations
More Info
Credits
Photo by Birmingham Museums Trust on Unsplash
This tweet was inspired by @Jan Giacomelli
Complexity is a sign of technical immaturity. Simplicity of use is the real sign of a well design product whether it is an ATM or a Patriot missile.
Daniel T. Ling
Software Engineering Great Quotes
Code Smell 107 - Variables Reuse
Reusing variables makes scopes and boundaries harder to follow
TL;DR: Don't read and write the same variable for different purposes
Problems
- Readability
- Hidden problems
Solutions
- Don't reuse variables
- Extract Method to isolate scopes
Context
When programming a script it is common to reuse variables.
This leads to confusion and makes debugging harder.
We should narrow the scope as much as possible.
Sample Code
Wrong
// print line total
double total = item.getPrice() * item.getQuantity();
System.out.println("Line total: " + total );
// print amount total
total = order.getTotal() - order.getDiscount();
System.out.println( "Amount due: " + total );
// variable is reused
Right
function printLineTotal() {
double total = item.getPrice() * item.getQuantity();
System.out.println("Line total: " + total );
}
function printAmountTotal() {
double total = order.getTotal() - order.getDiscount();
System.out.println( "Amount due: " + total );
}
Detection
- [ ]Automatic
Linters can use the parse tree to find variable definition and usages.
Tags
- Readability
Conclusion
Avoid reusing variable names. Use more specific and different names.
Relations
Code Smell 03 - Functions Are Too Long
More Info
Refactoring 002 - Extract Method
Credits
Simplicity before generality, use before reuse.
Kevlin Henney
Software Engineering Great Quotes
Code Smell 108 - Float Assertions
Asserting two float numbers are the same is a very difficult problem
TL;DR: Don't compare floats
Problems
- Wrong test results
- Fragile tests
- Fail fast principle violation
Solutions
- Avoid floats unless you have REAL performance concerns
- Use arbitrary precision numbers
- If you need to compare floats compare with tolerance.
Context
Comparing float numbers is an old computer science problem.
The usual solution is to use threshold comparisons.
We recommend avoiding floats at all and trying to use infinite precision numbers.
Sample Code
Wrong
Assert.assertEquals(0.0012f, 0.0012f); // Deprecated
Assert.assertTrue(0.0012f == 0.0012f); // Not JUnit - Smell
Right
Assert.assertEquals(0.0012f, 0.0014f, 0.0002); // true
Assert.assertEquals(0.0012f, 0.0014f, 0.0001); // false
// last parameter is the delta threshold
Assert.assertEquals(12 / 10000, 12 / 10000); // true
Assert.assertEquals(12 / 10000, 14 / 10000); // false
Detection
- [ ]Automatic
We can add a check con assertEquals() on our testing frameworks to avoid checking for floats.
Tags
- Test Smells
Conclusion
We should always avoid comparing floats.
Relations
Code Smell 71 - Magic Floats Disguised as Decimals
More Info
Credits
Photo by Mika Baumeister on Unsplash
God made the natural numbers; all else is the work of man.
Leopold Kronecker
Software Engineering Great Quotes
Code Smell 109 - Automatic Properties
What happens if you combine 4 code smells?
TL;DR: Avoid Getters, Avoid Setters, Avoid Metaprogramming. Think about Behavior.
Problems
- Information Hiding Violation
- Mutability
- Fail Fast Principle violation
- Duplicate code when setting properties
Solutions
Context
Setters and getters are a bad industry practice.
Many IDEs favor this code smell.
Some languages provide explicit support to build anemic models and DTOs.
Sample Code
Wrong
class Person
{
public string name
{ get; set; }
}
Right
class Person
{
private string name
public Person(string personName)
{
name = personName;
//imutable
//no getters, no setters
}
//... more protocol, probably accessing private variable name
}
Detection
- [ ]Automatic
This is a language feature.
We should avoid immature languages or forbid their worst practices.
Tags
- Encapsulation
Conclusion
We need to think carefully before exposing our properties.
The first step is to stop thinking about properties and focus solely on behavior.
Relations
Code Smell 70 - Anemic Model Generators
More Info
- W3 schools
- Laziness I - Metaprogramming
- Laziness II - Code Wizards
- Refactoring 001 - Remove Setters
- The Evil Power of Mutants
- Fail Fast
Credits
Nothing is harder than working under a tight deadline and still taking the time to clean up as you go.
Kent Beck
Software Engineering Great Quotes
Code Smell 110 - Switches With Defaults
Default means 'everything we don't know yet'. We cannot foresee the future.
TL;DR: Don't add a default clause to your cases. Change it for an exception. Be Explicit.
Problems
- Coupling
- Fail Fast principle violation
- Open closed principle violation
Solutions
- Replace if and cases with polymorphism
- Change Default code to an Exception
Context
When using cases, we usually add a default case so it doesn't fail.
Failing is always better than taking decisions without evidence.
Since case and switches are also an smell, we can avoid them.
Sample Code
Wrong
switch (value) {
case value1:
// if value1 matches the following will be executed..
doSomething();
break;
case value2:
// if value2 matches the following will be executed..
doSomethingElse();
break;
default:
// if value does not presently match the above values
// or future values
// the following will be executed
doSomethingSpecial();
break;
}
Right
switch (value) {
case value1:
// if value1 matches the following will be executed..
doSomething();
break;
case value2:
// if value2 matches the following will be executed..
doSomethingElse();
break;
case value3:
case value4:
// We currently know these options exist
doSomethingSpecial();
break;
default:
// if value does not match the above values we need to take a decision
throw new Exception('Unexpected case ' + value + ' we need to consider it');
break;
}
Detection
- [x]Semi Automatic
We can tell our linters to warn us on default uses unless there's an exception.
Tags
- Fail Fast
Conclusion
Writing robust code doesn't mean we need to take decisions without evidence.
Relations
Code Smell 36 - Switch/case/elseif/else/if statements
More Info
Credits
Photo by Joshua Woroniecki on Unsplash
The cost of adding a feature isn’t just the time it takes to code it. The cost also includes the addition of an obstacle to future expansion. The trick is to pick the features that don’t fight each other.
John Carmack
Software Engineering Great Quotes
And that’s all for now…
The next article will explain 5 more code smells!