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:
Here is a step-by-step demonstration of fixing our metric test module using this approach in the next section.
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:
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))))
The complexity of the function is now reduced to 4
, which is manageable.
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:
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.
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.
18.219.4.174