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. Therefore, they are not required to be 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

• Part XXII

• Part XXIII

• Part XXIV

• Part XXV

• Part XXVI

  
  

Let's continue...

Code Smell 131 - Zero Argument Constructor

Objects created without arguments are often mutable and erratic

TL;DR: Pass all your essential arguments when creating objects.

Problems

• Mutability
• Anemic Models

Solutions

1. Use one complete and single constructor.
2. Avoid Setters and Getters

Context

It is common usage using a zero-argument constructor and a bunch of setters to change it.

Beans is a well-known example of this code smell.

Sample Code

Wrong

 public Person();

// Anemic and mutable

Right

public Person(String name, int age) {
     this.name = name;
     this.age = age;
     } 
 }

// We 'pass' the essence to the object 
// So it does not mutate

Detection

• [x]Automatic

We can check all constructors, but there are some false positives.

Stateless objects are a valid example.

Tags

• Mutability

Conclusion

Empty constructors are mutability hints and accidental implementation issues.

We need to research usages to improve our solutions.

Relations

Code Smell 68 - Getters

Code Smell 28 - Setters

Code Smell 01 - Anemic Models

Code Smell 40 - DTOs

More Info

• Nullary Constructor
• Nude Models — Part I : Setters
• Nude Models — Part II : Getters
• The Evil Power of Mutants

Credits

Photo by Ade Adebowale on Unsplash

Don't worry about design, if you listen to your code a good design will appear...Listen to the technical people. If they are complaining about the difficulty of making changes, then take such complaints seriously and give them time to fix things.

Martin Fowler

Software Engineering Great Quotes

Code Smell 132 - Exception Try Too Broad

Exceptions are handy. But should be as narrow as possible

TL;DR: Be as specific as possible when handling errors.

Problems

• Fail fast principle violation
• Missing errors
• False negatives

Solutions

1. Narrow the exception handler as much as possible

Sample Code

Wrong

import calendar, datetime
try: 
    birthYear= input('Birth year:')
    birthMonth= input('Birth month:')
    birthDay= input('Birth day:')
    # we don't expect the above to fail
    print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))
except ValueError as e:
    if str(e) == 'month must be in 1..12': 
        print('Month ' + str(birthMonth) + ' is out of range. The month must be a number in 1...12')
    elif str(e) == 'year {0} is out of range'.format(birthYear): 
        print('Year ' + str(birthYear) + ' is out of range. The year must be a number in ' + str(datetime.MINYEAR) + '...' + str(datetime.MAXYEAR))
    elif str(e) == 'day is out of range for month': 
        print('Day ' + str(birthDay) + ' is out of range. The day must be a number in 1...' + str(calendar.monthrange(birthYear, birthMonth)))

Right

import calendar, datetime

# We might add specialized tries dealing with errors from the following 3 statements
birthYear= input('Birth year:')
birthMonth= input('Birth month:')
birthDay= input('Birth day:')
# try scope should be narrow
try: 
    print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))
except ValueError as e:
    if str(e) == 'month must be in 1..12': 
        print('Month ' + str(birthMonth) + ' is out of range. The month must be a number in 1...12')
    elif str(e) == 'year {0} is out of range'.format(birthYear): 
        print('Year ' + str(birthYear) + ' is out of range. The year must be a number in ' + str(datetime.MINYEAR) + '...' + str(datetime.MAXYEAR))
    elif str(e) == 'day is out of range for month': 
        print('Day ' + str(birthDay) + ' is out of range. The day must be a number in 1...' + str(calendar.monthrange(birthYear, birthMonth)))

Detection

• [x]Manual

If we have a good enough test suite, we can perform mutation testing to narrow the exception scope as much as possible.

Tags

• Exceptions

Conclusion

We must make exceptions as surgical as possible.

Relations

Code Smell 26 - Exceptions Polluting

Code Smell 73 - Exceptions for Expected Cases

Credits

Photon from Jakob Braun on Unsplash

The primary duty of an exception handler is to get the error out of the lap of the programmer and into the surprised face of the user.

Verity Stob

Software Engineering Great Quotes

Code Smell 133 - Hardcoded IF Conditions

Hard coding is fine. For a short period of time

TL;DR: Don't leave a hardcoded mess on IFs.

Problems

• Testability
• Hardcoded values
• Open/Closed Principle Violation

Solutions

1. Replace all IFs with a dynamic condition or polymorphism.

Context

Hard-coding iF conditions is great when doing Test-Driven Development.

We need to clean up stuff.

Sample Code

Wrong

private string FindCountryName (string internetCode)
{
  if (internetCode == "de")
    return "Germany";
  else if(internetCode == "fr") 
    return "France";
  else if(internetCode == "ar")
    return "Argentina";
    // lots of elses
  else
    return "Suffix not Valid";
}

Right

private string[] country_names = {"Germany", "France", "Argentina"} // lots more
private string[] Internet_code_suffixes= {"de", "fr", "ar" } // more
 
private Dictionary<string, string> Internet_codes = new Dictionary<string, string>();

// There are more efficient ways for collection iteration
// This pseudocode is for illustration
int currentIndex = 0; 
foreach (var suffix in Internet_code_suffixes) {
  Internet_codes.Add(suffix, Internet_codes[currentIndex]);
  currentIndex++;
}

private string FindCountryName(string internetCode) {
  return Internet_codes[internetCode];
}

Detection

• [x]Automatic

By checking If/else conditions we can detect hard-coded conditions.

Tags

• IFs

Conclusion

In the past, hard-coding was not an option.

With modern methodologies, we learn by hard-coding, and then, we generalize and refactor our solutions.

Relations

Code Smell 36 - Switch/case/elseif/else/if statements

Code Smell 102 - Arrow Code

More Info

• How to Get Rid of IFs forever
• Test Driven Development

Credits

Photo by Jessica Johnston on Unsplash

Don't be (too) clever. My point was to discourage overly clever code because "clever code" is hard to write, easy to get wrong, harder to maintain, and often no faster than simpler alternatives because it can be hard to optimize.

Bjarne Stroustrup

Software Engineering Great Quotes

Code Smell 134 - Specialized Business Collections

If it walks like a duck and it quacks like a duck, then it must be a duck

TL;DR: Don't create unnecessary abstractions

Problems

• Over Design
• Unneeded classes

Solutions

1. Use a standard class

Context

Discovering abstractions on the MAPPER is a hard task.

After refining we should remove unneeded abstractions.

Sample Code

Wrong

<?php

Namespace Spelling;

final class Dictionary {

    private $words;
    function __construct(array $words) {
        $this->words = $words;
    }

    function wordsCount(): int {
        return count($this->words);
    }

    function includesWord(string $subjectToSearch): bool {
        return in_array($subjectToSearch, $this->words);
    }
}

// This has protocol similar to an abstract datatype dictionary
// And the tests
  
use PHPUnit\Framework\TestCase;

final class DictionaryTest extends TestCase {
    public function test01EmptyDictionaryHasNoWords() {
        $dictionary = new Dictionary([]);
        $this->assertEquals(0, $dictionary->wordsCount());
    }

    public function test02SingleDictionaryReturns1AsCount() {        
        $dictionary = new Dictionary(['happy']);
        $this->assertEquals(1, $dictionary->wordsCount());
    }

    public function test03DictionaryDoesNotIncludeWord() {
        $dictionary = new Dictionary(['happy']);
        $this->assertFalse($dictionary->includesWord('sadly'));
    }

    public function test04DictionaryIncludesWord() {
        $dictionary = new Dictionary(['happy']);
        $this->assertTrue($dictionary->includesWord('happy'));
    }
} 

Right

<?php

Namespace Spelling;

// final class Dictionary is no longer needed
    
// The tests use a standard class
// In PHP we use associative arrays
// Java and other languages have HashTables, Dictionaries etc. etc.
  
use PHPUnit\Framework\TestCase;

final class DictionaryTest extends TestCase {
    public function test01EmptyDictionaryHasNoWords() {
        $dictionary = [];
        $this->assertEquals(0, count($dictionary));
    }

    public function test02SingleDictionaryReturns1AsCount() {
        $dictionary = ['happy']; 
        $this->assertEquals(1, count($dictionary));
    }

    public function test03DictionaryDoesNotIncludeWord() {
        $dictionary = ['happy']; 
        $this->assertFalse(in_array('sadly', $dictionary));
    }

    public function test04DictionaryIncludesWord() {
        $dictionary = ['happy'];  
        $this->assertTrue(in_array('happy', $dictionary));
    }
} 

Detection

• [x]Semi-Automatic

Based on protocols, we should remove some unnecessary classes

Exceptions

Sometimes we need to optimize collections for performance reasons if we have enough strong evidence.

Tags

• Protocols

Conclusion

We need to clean up code from time to time.

Specialized collections are a good starting point.

Relations

Code Smell 111 - Modifying Collections While Traversing

More Info

• Duck Typing

Credits

Photo by Pisit Heng on Unsplash

Most of the effort in the software business goes into the maintenance of code that already exists.

Wietse Venema

Software Engineering Great Quotes

Code Smell 135 - Interfaces With just One Realization

Being generic and foreseeing the future is good.

TL;DR: Don't over-generalize

Problems

• Speculative Design
• Complexity
• Over-Engineering

Solutions

1. Remove the interface until you get more examples

Context

In the past, programmers told us to design for change.

Nowadays, We follow the scientific method.

Whenever we find a duplication we remove it.

Not before.

Sample Code

Wrong

public interface Vehicle {
    public void start();
    public void stop();
}

public class Car implements Vehicle {
    public void start() {
        System.out.println("Running...");
    }
    public void stop() {
        System.out.println("Stopping...");
    }
}

// No more vehicles??

Right

public class Car {
    public void start() {
        System.out.println("Running...");
    }
    public void stop() {
        System.out.println("Stopping...");
    }
}

// Wait until more vehicles are discovered

Detection

• [x]Automatic

This is very easy for our linters since they can trace this error at compile time.

Exceptions

This rule applies to inter system definition and business logic.

Some frameworks define an Interface as protocol to be fulfilled.

On our bijections we need to model existing real-world protocols.

Interfaces are the MAPPER correspondence to protocol.

Dependency injection/Invesion protocols declare interfaces that are fulfilled with their realizations. Until then, they can be empty.

If your language defines an interface for test mocking, it is another code smell.

Tags

• Over Design

Relations

Code Smell 130 - AddressImpl

Code Smell 30 - Mocking Business

Code Smell 136 - Classes With just One Subclass

Conclusion

We need to wait for abstractions and not be creative and speculative

Credits

Photo by Brian Kostiuk on Unsplash

I love software, because if you can imagine something, you can build it.

Ray Ozzie

Software Engineering Great Quotes

And that’s all for now…

The next article will explain five more code smells!