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

1. 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 <[email protected]>")
  else:
    print("Emails are sent only on production")
    
send_welcome_email("[email protected]", "development")
# Emails are sent only on production

send_welcome_email("[email protected]", "production")
# Sending welcome email to [email protected] from Bob Builder <[email protected]>

Right

class ProductionEnvironment:
  FROM_EMAIL = "Bob Builder <[email protected]>"

class DevelopmentEnvironment:
  FROM_EMAIL = "Bob Builder Development <[email protected]>"
  
# 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("[email protected]", DevelopmentEnvironment())
# Sending welcome email to [email protected] from Bob Builder Development <[email protected]>

send_welcome_email("[email protected]", ProductionEnvironment())
# Sending welcome email to [email protected] from Bob Builder <[email protected]>

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

Code Smell 56 - Preprocessors

More Info

• How to avoid annoying ifs

Credits

Photo by on This tweet was inspired by @

---

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

1. Don't reuse variables
2. 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

Photo by on

---

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

1. Avoid floats unless you have REAL performance concerns
2. Use arbitrary precision numbers
3. 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

• Fail fast

Credits

Photo by on

---

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

1. Remove automatic setters and getters

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 28 - Setters

Code Smell 68 - Getters

Code Smell 70 - Anemic Model Generators

Code Smell 40 - DTOs

Code Smell 01 - Anemic Models

More Info

• Laziness I - Metaprogramming
• Laziness II - Code Wizards
• Refactoring 001 - Remove Setters
• The Evil Power of Mutants
• Fail Fast

Credits

Photo by on

---

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

1. Replace if and cases with polymorphism
2. 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

• Fail Fast

Credits

Photo by on

---

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!