Refactoring: Extract method

Extract method is one of the most popular refactoring method when dealing with legacy code. In legacy code we can often find methods that are very long. My favorite findings methods about 2000 lines of code. Cool, isn’t it? Those methods have usually many responsibilities and they are hard to debug. Having more than one responsibility in one method leads also to duplicated code because some responsibility is required in more than one place in code.

As an example let’s see the following code written in PHP.

function get_active_users()
{
    // find active users
    $query = mysql_query("select * from plah where id=$id");
    while($result = mysql_fetch_assoc($query))
    {
        if($result[‘sec_code’]==security_code($result[‘id’]))
            $results[] = $result;
    }

    // create options array
    $options = array();
    foreach($results as $val)
    {
        $optid = $val[‘id’];
        $opttext = $val[‘title’];
        $options[] = "<option value=’$optid’>$opttext</option>";
   }
    return $options;
}

This method is useful for sure and it works like expected but it does more then expected. What if we want to use array of active users elsewhere in the code? This may be not new need. We have to search through code to see if this code is duplicated also in some other method.

What we have to do is to move code that finds active users to another method. This way we have one method that returns users array and the other that creates list of options based on it. After extracting active users finding code to another method we have code like this.

function get_active_users()
{
    $query = mysql_query("select * from plah where id=$id");
  
    while($result = mysql_fetch_assoc($query))
    {
        if($result[‘sec_code’]==security_code($result[‘id’]))
            $results[] = $result;
    }

    return $results;
}

function get_active_users_options()
{
    $active_users = get_active_users();
    $options = array();
  
    foreach($active_users as $val)
    {
        $optid = $val[‘id’];
        $opttext = $val[‘title’];
        $options[] = "<option value=’$optid’>$opttext</option>";
    }

    return $options;
}

Now we have two methods instead of one. This may seem like bad idea because the number of methods grows. But there is no problem because we have now two methods and both of them have only one responsibility. All we have to do now is to find out other parts in code where list of active users is needed and replace the code with method call. When logic of finding active users changes we have to make the change only in one method.

Book recommendation! Those who want to find out more about refactoring and have a timeless hardcover handbook on the work desk should consider the book “Refactoring: Improving the Design of Existing Code” by Martin Fowler.