Refactoring code

Now that we have seen how static checkers can be used to report a wide range of errors and issues in our Python code, let's do a simple exercise of refactoring our code. We will take our poorly written metric test module as the use case (the first version of it) and perform a few refactoring steps.

Here are the rough guidelines to follow when refactoring software:

1. Fix complex code first: This will get a lot of code out of the way as typically, when a complex piece of code is refactored, we end up reducing the number of lines of code. This overall improves the code quality and reduces code smells. You may be creating new functions or classes here, so it always helps to perform this step first.
2. Do an analysis of the code: It is a good idea to run the complexity checkers at this step and see how the overall complexity of the code—class/module or functions—has been reduced. If not, iterate again.
3. Fix code smells next: Fix any issue with code smells—class, function, or module—next. This gets your code into a much better shape and improves the overall semantics.
4. Run checkers: Run checkers such as Pylint on the code now, and get a report on the code smells. Ideally, they should be close to zero or reduced very much from the original.
5. Fix low-hanging fruits: Fix low-hanging fruits, such as code style and convention errors, last. This is because, in the process of refactoring, when trying to reduce complexity and code smells, you typically would introduce or delete a lot of code. So, it doesn't make sense to try and improve the code convention issues at earlier stages.
6. Perform a final check using the tools: You can run Pylint for code smells, Flake8 for PEP-8 conventions, and Pyflakes for catching the logic, syntax, and missing variable issues.

Here is a step-by-step demonstration of fixing our metric test module using this approach in the next section.

Refactoring code – fixing complexity

Most of the complexity is in the office route function, so let's try and fix it. Here is the rewritten version (showing only that function here):

def find_optimal_route_to_my_office_from_home(start_time,
                                              expected_time,
                                              favorite_route='SBS1K',
                                              favorite_option='bus'):
                                                 
        d = (expected_time - start_time).total_seconds()/60.0

        if d<=30:
            return 'car'
        elif d<45:
            return ('car', 'metro')
        elif d<60:
            # First volvo,then connecting bus
            return ('bus:335E','bus:connector')
        elif d>80:
            # Might as well go by normal bus
            return random.choice(('bus:330','bus:331',':'.join((favorite_option,
                                         favorite_route))))
        # Relax and choose favorite route
        return ':'.join((favorite_option, favorite_route))

In the preceding rewrite, we got rid of the redundant if...else conditions. Let's check the complexity now:

mccabe metric of metric test program after refactoring step 1

We were able to reduce the complexity from 7 to 5. Can we do better?

In the following piece of code, the code is rewritten to use ranges of values as keys, and the corresponding return value as values. This simplifies our code a lot. Also, the earlier default return at the end would never have got picked, so it is removed now, hence getting rid of a branch and reducing complexity by one. The code has become much simpler:

def find_optimal_route_to_my_office_from_home(start_time,
                                              expected_time,
                                              favorite_route='SBS1K',
                                              favorite_option='bus'):

    # If I am very late, always drive.
    d = (expected_time – start_time).total_seconds()/60.0
    options = { range(0,30): 'car',
    range(30, 45): ('car','metro'),
    range(45, 60): ('bus:335E','bus:connector') }


if d<80:
# Pick the range it falls into
for drange in options:
    if d in drange:
    return drange[d]

    # Might as well go by normal bus
    return random.choice(('bus:330','bus:331',':'.join((favorite_option, favorite_route))))
 

mccabe metric of metric test program after refactoring step #2

The complexity of the function is now reduced to 4, which is manageable.

Refactoring code – fixing code smells

The next step is to fix code smells. Thankfully, we have a very good list from the previous analysis, so this is not too difficult. Mostly, we need to change function names and variable names and fix the contracts from child class to parent class.

Here is the code with all of the fixes:

""" Module metrictest.py - testing static quality metrics of Python code """

import random

def sum_fn(xnum, ynum):
    """ A function which performs a sum """

    return xnum + ynum

def find_optimal_route(start_time,
                       expected_time,
                       favorite_route='SBS1K',
                       favorite_option='bus'):
    """ Find optimal route for me to go from home to office """
    
    # Time difference in minutes - inputs must be datetime instances
    tdiff = (expected_time - start_time).total_seconds()/60.0

    options = {range(0, 30): 'car',
               range(30, 45): ('car', 'metro'),
               range(45, 60): ('bus:335E', 'bus:connector')}


    if tdiff < 80:
        # Pick the range it falls into
        for drange in options:
            if tdiff in drange:
                return drange[tdiff]
            
    # Might as well go by normal bus
    return random.choice(('bus:330', 'bus:331', ':'.join((favorite_option,
                                    favorite_route))))

    
class MiscClassC(object):
    """ A miscellaneous class with some utility methods """

    def __init__(self, xnum, ynum):
        self.xnum = xnum
        self.ynum = ynum
        
    def compare_and_sum(self, xnum=0, ynum=0):
        """ Compare local and argument variables
        and perform some sums """
        
        if self.xnum > xnum:
            return self.xnum + self.ynum
        else:
            return xnum + self.ynum

class MiscClassD(MiscClassC):
    """ Sub-class of MiscClassC overriding some methods """

    def __init__(self, xnum, ynum=0):
        super(MiscClassD, self).__init__(xnum, ynum)

    def some_func(self, xnum, ynum):
        """ A function which does summing """
        
        if xnum > ynum:
            return xnum - ynum
        else:
            return xnum + ynum

    def compare_and_sum(self, xnum=0, ynum=0):
        """ Compare local and argument variables
        and perform some sums """

        if self.xnum > ynum:
            return self.xnum + ynum
        else: 
            return ynum - self.xnum

Let's run Pylint on this code and see what it outputs this time:

Pylint output of refactored metric test program

You see that the number of code smells has boiled down to near zero except a complaint of lack of public methods, and the insight that the some_func method of the MiscClassD class can be a function, as it does not use any attributes of the class.

Note

We have invoked Pylint with the –reports=n option in order to avoid Pylint printing its summary report, as it would make the entire output too long to display here. These reports can be enabled by calling Pylint without any argument.

Refactoring code – fixing styling and coding issues

Now that we have fixed the major code issues, the next step is to fix code style and convention errors. However, in order to shorten the number of steps and the amount of code to be printed in this book for this exercise, this was already merged along with the last step, as you may have guessed from the output of Pylint.

Except for a few whitespace warnings, all of the issues are fixed.

This completes our Refactoring exercise.