Yet more code smells? More? Where do they come from? We see several symptoms and situations that make us doubt the quality of our development. Let's look at some possible solutions. Most of these smells are just hints of something that might be wrong. They are not rigid rules. Previous Parts Part I Part II Part III Part IV Part V Part VI Part VII Part VIII Part IX Part X Part XI Let's continue... Code Smell 56 - Preprocessors We want our code to behave different on different environments, operating systems, so taking decisions at compile time is the best decision, isn't it?. Problems Readability Premature Optimization Unnecessary complexity Debugging Solutions Remove all compiler directives. If you want different behavior, model it with objects If you think there's a performance penalty, make a serious benchmark instead of doing premature optimization. Sample Code Wrong #if VERBOSE >= 2
  printf("trace message");
#endif Right if (runtimeEnvironment->traceDebug()){
  printf("trace message");
}

## even better with polymorphism and we avoid annoying ifs

runtimeEnvironment->traceDebug("trace message"); Detection This is a syntactic directive promoted by several languages, therefore it is easy to detect and replace with real behavior. Tags Compilers Metaprogramming Conclusion Adding an extra layer of complexity makes debugging very difficult. This technique was used when memory and CPU were scarce. Nowadays, we need clean code, and we must leave premature optimization buried in the past. Bjarne Stroustrup, in his book , regrets on the pre-processor directives he created years before. The Design and Evolution of C++ More info Laziness Chapter I: Meta-Programming C++ FAQ Wikipedia #ifdef Considered Harmful C++ is designed to allow you to express ideas, but if you don't have ideas or don't have any clue about how to express them, C++ doesn't offer much help. Bjarne Stroustrup Code Smell 57 - Versioned Functions sort, sortOld, sort20210117, workingSort, It is great to have them all. Just in case Problems Readability Maintainability Solutions Keep just one working version of your artefact (class, method, attribute). Leave time control to your version control system. Sample Code Wrong findMatch()
findMatch_new()
findMatch_newer()
findMatch_newest()
findMatch_version2()
findMatch_old()
findMatch_working()
findMatch_for_real()
findMatch_20200229()
findMatch_thisoneisnewer()
findMatch_themostnewestone()
findMatch_thisisit()
findMatch_thisisit_for_real() Right findMatch() Detection We can add automatic rules to find versioned methods with patterns. Like many other patters we might create an internal policy and communicate. Tags Versioning Conclusion Time and code evolution management is always present in software development. Luckily nowadays we have mature tools to address this problem. Credits Original idea That's why I write, because life never works except in retrospect. You can't control life, at least you can control your version. Chuck Palahniuk Code Smell 58 - Yo-yo Problem Searching for a concrete method implementation? Go back and forth, up and down. TL;DR: Don't ab(use) hierarchies. Problems Deep Hierarchies Subclassification for Code Reuse Readability Low Cohesion High Coupling Solutions Favor composition over inheritance. Refactor deep hierarchies. Sample Code Wrong <?

abstract class Controller {

}

class BaseController extends Controller {

}

class SimpleController extends BaseController {

}

class ControllerBase extends SimpleController {

}

class LoggedController extends ControllerBase {

}

class RealController extends LoggedController {

} Right <?

interface ControllerInterface {
  
}

abstract class Controller implements ControllerInterface {

}

final class LoggedControllerDecorator implements ControllerInterface {

}

final class RealController implements ControllerInterface {

} Detection Any linter can check for suspects against a max depth threshold. Tags Hierarchy Conclusion Many novice programmers reuse code through hierarchies. This brings high coupled and low cohesive hierarchies. established in their paper this was actually a good design recipe back in 1988. We have learned a lot from there. Johnson and Foote We must refactor and flatten those classes. More info https://en.wikipedia.org/wiki/Yo-yo_problem Hierarchies should be deep by Johnson and Foote An error arises from treating object variables (instance variables) as if they were data attributes and then creating your hierarchy based on shared attributes. Always create hierarchies based on shared behaviors, side. David West Code Smell 59 - Basic / Do Functions sort, doSort, basicSort, doBasicSort, primitiveSort, superBasicPrimitiveSort, who does the real work? TL;DR: Shortcuts for mini wrappers shout for better solutions. Problems Readability Bad Naming Low Cohesion Single Responsibility Principle Solutions Use good object wrappers Use dynamic decorators Sample Code Wrong <?

final class Calculator {

    private $cachedResults;

    function computeSomething() {
        if (isset($this->cachedResults)) {
            return $this->cachedResults;
        }
        $this->cachedResults = $this->logAndComputeSomething();
    }

    private function logAndComputeSomething() {
        $this->logProcessStart();
        $result = $this->basicComputeSomething();
        $this->logProcessEnd();
        return $result;
    }

    private function basicComputeSomething() {
        /// Do Real work here
    }

} Right <?

final class Calculator {
    function computeSomething() {
        // Do Real work here since I am Compute!
    }
}

//Clean and cohesive class, single responsibility

final class CalculatorDecoratorCache {

    private $cachedResults;
    private $decorated;

    function computeSomething() {
        if (isset($this->cachedResults)) {
            return $this->cachedResults;
        }
        $this->cachedResults = $this->decorated->computeSomething();
    }
}

final class CalculatorDecoratorLogger {

    private $decorated;

    function computeSomething() {
        $this->logProcessStart();
        $result = $this->decorated->computeSomething();
        $this->logProcessEnd();
        return $result;
    }
} Detection We can instruct our static linters to find wrapping methods if they follow conventions like , etc. doXXX() basicXX() Tags Declarativiness Conclusion We came across this kind of methods some time in our developer life, We smelled something was not OK with them. Now is the time to change them! More info https://en.wikipedia.org/wiki/Wrapper_function https://en.wikipedia.org/wiki/Decorator_pattern The primary disadvantage of Wrap Method is that it can lead to poor names. In the previous example, we renamed the pay method dispatchPay() just because we needed a different name for code in the original method. Michael Feathers Code Smell 60 - Global Classes Classes are handy. We can call them and invoke them any time. Is this good? TL;DR: Don't use your classes as a global point of access. Problems Coupling Classes are global unless we use Namespaces. Name polluting Static Methods Static Constants Singletons Solutions Use namespaces, module qualifiers or similar To avoid namespace polluting, keep the Global names as short as possible. Class single Responsibility is to create instances. Sample Code Wrong <?

final class StringUtilHelper {
    static function reformatYYYYDDMMtoYYYYMMDD($date) {
    }
}

class Singleton {

}

final class DatabaseAccessor extends Singleton {

} Right <?

namespace Date;

final class DateFormatter {

    function reformatYYYYDDMMtoYYYYMMDD(Date $date) {
    }
    //function is not static since class single responsibility is to create instances and not be a library of utils

}

namespace OracleDatabase;

class DatabaseAccessor {
    //Database is not a singleton and it is namespace scoped
} Detection We can use almost any linter or create dependency rules searching for bad class references. Tags Globals Conclusion We should restrict our classes to small domains and expose just facades to the outside. This greatly reduces coupling. More info Singleton the root of all evil Coupling: The one and only software design problem Write shy code — modules that don't reveal anything unnecessary to other modules and that don't rely on other modules' implementations. Dave Thomas We are done for some time (not many). But we are pretty sure we will come across even more smells very soon! I keep getting more suggestions on , so they won't be the last! twitter Send me your own!

Trace

Twitter

How to Find The Stinky Parts of Your Code [Part XIII]

Software Engineer Maxi Contieri on writing about “boring” fundamentals 

Buy My Book

Share your ideas with me!

Read My Stories

Buy Me a Coffee

Too Long; Didn't Read

Make resilience your competitive advantage

How to Find The Stinky Parts of Your Code [Part XII]

How to Find The Stinky Parts of Your Code [Part XII]

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

Untitled Story

Code Smell 309 - Query Parameter API Versioning

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

Code Smell 309 - Query Parameter API Versioning

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