Fellipe Sanches' website

Category: Refactoring

  • Code smell: how to fix Long Methods

    Long Method is our code smell type today.

    What is a code smell?

    A code smell is a sign that something in your code might be poorly designed and could cause problems later on.

    In this series of posts I will talk about the most common code smells problems and then how fix each one.

    Problem:

    A method with many lines of code, or not in the best-case, but which is still difficult to understand, reuse, and maintain.

    Ways to avoid it:

    Divide your confusing method into well-defined part

    You have a block of code that logically belongs together:

    //Bad :-(
    //Everything mixed together
    function showProfile() {
        $this->showHeader();
    
        // All profile details printed here
        echo "User: " . $this->username;
        echo "Email: " . $this->email;
    }

    Extract it into a new method and replace the original code with a call to that method:

    //Good :-)
    //Details moved to another function
    function showProfile() {
        $this->showHeader();
        $this->printUserInfo();
    }
    
    function printUserInfo() {
        echo "User: " . $this->username;
        echo "Email: " . $this->email;
    }

    If you change the user info, you only change one function, and you can reuse it.

    Change temporary variables by a query

    Sometimes, you create a variable just to store a value temporarily so you can use it later in your code. This makes the code harder to reuse, harder to clean up, and harder to extract into smaller methods:

    //Bad :-(
    //Using a temp variable
    $totalCost = $this->cups * $this->pricePerCup;
    
    if ($totalCost > 20) {
        return "You get a free cookie!";
    } else {
        return "No free cookie this time.";
    }
    

    Create a small function that can calculate the value anytime you ask:

    //Good :-)
    //Replace temp var with query
    if ($this->totalCost() > 20) {
        return "You get a free cookie!";
    } else {
        return "No free cookie this time.";
    }
    
    function totalCost() {
        return $this->cups * $this->pricePerCup;
    }
    

    The total cost is calculated in one single place, where you can change the price or rule.

    Change parameters that are repeated by an object

    Your methods contain a repeating group of parameters:

    class Customer
    {
        public function amountInvoicedIn(DateTime $start, DateTime $end) {
            // ...
        }
    
        public function amountReceivedIn(DateTime $start, DateTime $end) {
            // ...
        }
    
        public function amountOverdueIn(DateTime $start, DateTime $end) {
            // ...
        }
    }

    Replace these parameters with an object:

    class Customer
    {
        public function amountInvoicedIn(DateRange $range) {
            // ...
        }
    
        public function amountReceivedIn(DateRange $range) {
            // ...
        }
    
        public function amountOverdueIn(DateRange $range) {
            // ...
        }
    }
    

    Less repetition, cleaner code, and the ability to make changes within the object in a centralized way.

    Preserve whole object

    You write a method that needs several pieces of data from the same object, you pass three or four values taken from the same object for instance just to call another method:

    $startDatetime = $today->getStartDatetime();
    $endDatetime = $today->getEndDatetime();
    $todaysTime = $time->timeDiff($startDatetime, $endDatetime);

    Instead of doing that, you can simply pass the entire object and let the method get whatever it needs:

    $todaysTime = $time->timeDiff($today);