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

Let's continue...

Code Smell 91 - Test Asserts without Description

We are big fans of xUnit. But we don't care much for the programmers

TL;DR: Use asserts with declarative descriptions.

Problems

• Readability
• Hard debugging
• Time waste

Solutions

1. Put a nice descriptive assertion
2. Share guides for problem-solving

Sample Code

Wrong

<?

public function testNoNewStarsAppeared(): void
  {
     $expectedStars = $this->historicStarsOnFrame();
     $observedStars = $this->starsFromObservation();
     //These sentences get a very large collection
  
     $this->assertEquals($expectedStars, $observedStars);
     //If something fails we will have a very hard debugging time
    }

Right

<?

public function testNoNewStarsAppeared(): void
  {
     $expectedStars = $this->historicStarsOnFrame();
     $observedStars = $this->starsFromObservation();
     //These sentences get a very large collection
  
     $newStars = array_diff($expectedStars, $observedStars);
  
     $this->assertEquals($expectedStars, $observedStars ,
         'There are new stars ' . print_r($newStars,true));
     //Now we can see EXACTLY why the assertion failed with a clear and
     //Declarative Message 
    }

Detection

Since assert and assertDescription are different functions, we can adjust our policies to favour the latter.

Tags

• Test Smells

Conclusion

Be respectful to the reader of your assertions.

It might even be yourself!

More Info

• XUnit: Assert Description Deprecation

Credits

Photo by Startaê Team on Unsplash

Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.

John Woods

Code Smell 92 - Isolated Subclasses Names

If your classes are globals, use fully qualified names

TL;DR: Don't use abbreviations in subclasses

Problems

• Readability
• Mistakes

Solutions

1. Rename your classes to provide context
2. Use modules, namespaces or fully qualified names

Sample Code

Wrong

abstract class PerserveranceDirection { 
}

class North extends PerserveranceDirection {}
class East extends PerserveranceDirection {}
class West extends PerserveranceDirection {}
class South extends PerserveranceDirection {}

//Subclasses have short names and meaningless outside the hierarchy
//If we reference East we might mistake it for the Cardinal Point

Right

abstract class PerserveranceDirection { 
}

class PerserveranceDirectionNorth extends PerserveranceDirection {}
class PerserveranceDirectionEast extends PerserveranceDirection {}
class PerserveranceDirectionWest extends PerserveranceDirection {}
class PerserveranceDirectionSouth extends PerserveranceDirection {}

//Subclasses have fully quallified names

Detection

Automatic detection is not an easy task. We could enforce local naming policies for subclasses.

Tags

• Naming

Conclusion

Choose your names wisely.

If your language supports it, use modules, namespaces and local scopes.

Relations

Code Smell 11 - Subclassification for Code Reuse

More Info

• What is in a name?
• MAPPER

Credits

Photo by Edvard Alexander Rølvaag on Unsplash

The programmer's primary weapon in the never-ending battle against slow system is to change the intramodular structure. Our first response should be to reorganize the modules' data structures.

Frederick P. Brooks

Code Smell 93 - Send me Anything

Magic functions that can receive a lot of different (and not polymorphic arguments)

TL;DR: Create a clear contract. Expect just one protocol.

Problems

• Fail Fast principle violation
• Error prune
• Readability
• If polluting
• Nulls
• Bad Cohesion

Solutions

1. Take just one "kind" of input
2. Arguments should adhere to a single protocol.

Sample Code

Wrong

<?

function parseArguments($arguments) {
    $arguments = $arguments ?: null;
    //Always the billion-dollar mistake
    if (is_empty($arguments)) {
        $this->arguments = http_build_query($_REQUEST);
        //Global coupling and side effects
    } elseif (is_array($arguments)) {
        $this->arguments = http_build_query($arguments);
    } elseif (!$arguments) { //null unmasked
        $this->arguments = null;
    } else {
        $this->arguments = (string)$arguments;
    }
}

Right

<?

function parseArguments(array $arguments) {
    $this->arguments = $arguments;
    //much cleaner, isn't it ?
}

Detection

We can detect this kind of methods when they do different things, asking for the argument kind

Tags

• If Polluter

Conclusion

Magic castings and flexibility have a price. They put the rubbish under the rug and violate fail fast principle.

Relations

Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)

Code Smell 06 - Too Clever Programmer

Code Smell 12 - Null

Credits

Photo by Hennie Stander on Unsplash

Referential transparency is a very desirable property: it implies that functions consistently yield the same results given the same input, irrespective of where and when they are invoked.

Edward Garson

Code Smell 94 - Too Many imports

If your class relies on too many others, it will be coupled and fragile. A long import list is a good indicator.

TL;DR: Don't import too much.

Problems

• Coupling
• Single Responsibility Principle violation
• Low Cohesion

Solutions

1. Break the class
2. Hide intermediate accidental implementation

Sample Code

Wrong

import java.util.LinkedList;
import java.persistence;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.NoSuchElementException 
import java.util.Queue;
import org.fermi.common.util.ClassUtil;
import org.fermi.Data;
//We rely on too many libraries

public class Demo {
   public static void main(String[] args) {
      
   }
}

Right

import org.fermi.domainModel;
import org.fermi.workflow;

//We rely on few libraries
//and we hide their implementation
//So maybe transitive imports are the same
//but we don't break encapsulation

public class Demo {
   public static void main(String[] args) {
      
   }
}

Detection

We can set a warning threshold on our linters.

Tags

• Coupling
• Ripple Effect

Conclusion

We need to think about dependencies when building our solutions to minimize Ripple Effect.

Relations

Code Smell 61 - Coupling to Classes

More Info

• Coupling: The one and only software design problem
• Namespaces on Wikipedia

Credits

Photo by Zdeněk Macháček on Unsplash

Twitter

Fools ignore complexity. Pragmatists suffer it. Some can avoid it. Geniuses remove it.

Alan Perlis

Code Smell 95 - Premature Classification

We are over generalizers. We shouldn't create abstractions until we see enough concretions.

TL;DR: Don't guess what the future will bring you.

Context

Aristotelian Classification is a big problem in computer science. We tend to classify and name things before gathering enough knowledge and context.

Problems

• Futurology
• Bad designs

Solutions

1. Wait for concretions
2. Refactor late

Sample Code

Wrong

class Rectangle 
 { 
       int length; 
       int breadth; 
       
       int area() 
       {
         return length * breadth;
       } 
 } 
//We are creating a premature abstraction 
//And misusing is-a relation since a Square "is a" Rectangle

class Square extends Rectangle
 { 
       int length;  
       
       int area() 
       {  
         return length * length; 
       } 
 } 

Right

class Rectangle 
 { 
       int length; 
       int breadth; 
       
       int area() 
       {
         return length * breadth;
       } 
 }  

class Square 
{ 
       int length;  
       
       int area() 
       {  
         return length * length; 
       } 
 } 
//Square might-be a Rectangle
//But it does not follow behaves-like relation so we won't go ahead
//and create a strong relation between them
//Maybe they are shapes. We don't have enough examples and protocol yet
//We will not guess until further knowledge

Detection

An abstract class with just one subclass is an indicator of premature classification

Tags

• Bad Design
• Classification

Conclusion

When working with classes, we name abstractions as soon as they appear.

Our rule is to choose good names after the behaviour.

We should not name our abstractions until we name our concrete subclasses.

Relations

Code Smell 11 - Subclassification for Code Reuse

More Info

• What is in a name

Credits

Photo by Faye Cornish on Unsplash

Let us change our traditional attitude to the construction of programs: Instead of imagining that our main task is to instruct a computer what to do, let us concentrate rather on explaining to human beings what we want a computer to do.

Donald E. Knuth

Software Engineering Great Quotes

And that’s all for now…

The next article will explain 5 more code smells!