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 Put a nice descriptive assertion 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 and are different functions, we can adjust our policies to favour the latter. assert assertDescription 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 on Startaê Team 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 Rename your classes to provide context 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 on Edvard Alexander Rølvaag 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 Take just one "kind" of input 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 on Hennie Stander 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 Break the class 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 on Zdeněk Macháček 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 gathering enough knowledge and context. before Problems Futurology Bad designs Solutions Wait for concretions 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 after the behaviour. good names 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 on Faye Cornish 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!

Twitter

How to Find the Stinky Parts of Your Code [Part XX]: We have Reached 100!

How to Find the Stinky Parts of Your Code [Part XVIII]

Buy My Book

How to Find the Stinky Parts of Your Code [Part XIX]

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

Untitled Story

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

Light-Mode

Classic

Newspaper

Dark-Mode

Neon Noir

Minty

HN StartUps