Refactoring Python code in 5 simple steps4 min read

Refactoring is probably the most annoying and tedious process for any developer. It is understandable, nobody enjoys going back to “fix” something that works, right? Well, refactoring is not about you. No matter who originally had written the code, there will be someone responsible for maintaining and adding features to the code base and it might be you. So, do your future-self or colleague a favour and clean up the code so no one has to suffer after you.

In the last post, we tackled the challenge of writing tests for the Gilded Rose by Emily Bache. You can find the results on my GitHub or stick around to find out how to quickly refactor your code in 5 simple steps.

Step 0. To refactor or not to refactor…

This question tormented generations of developers. Is it worth trying to make sense of all these code noodles or do we rewrite the whole portion of the code? 

Before making a decision consider a few scenarios I encountered over the years:

If the code has a bug that crashes the app, it is often an issue hard to spot after it made its way into production. If you can’t spot the issue from the get-go it is unlikely refactoring will uncover it without causing additional problems. It is extremely hard to refactor broken code so try to avoid doing it.

If you want to just add a feature, it is often not that hard to just add it in isolation and then proceed with refactoring as we did in the code kata. This is the best-case scenario as you don’t have to tackle the legacy code.

If you tried to add the feature in isolation but it messes with the original one, there is likely a big design issue that needs a rewrite. Your code has to be modular with minimal dependencies.

Finally, if you decide that it is possible to refactor your code for future convenience proceed with the steps below.

Step 1. Extract logical tests and trivial operations

In gilded_rose_checks_operations_extracted.py you can see that all the logical tests and some operations were replaced with functions. This is done to make the writing faster in the future (thanks to autocompleting) and reduces redundancy.

The names of the functions are as descriptive as possible and execute ONE operation. Let’s take a look at an example:

if item.name != "Aged Brie":

Becomes

if itemNotBrieCheck(item):

def itemNotBrieCheck(item):
    return item.name != "Aged Brie"

I prefer using camelCase for the naming scheme and it is constructed as follows:

item refers item.name

Not refers  !=

Brie refers “Aged Brie”

Check indicates that this is a logical test we are going to use in an if-statement

With this naming scheme, it is easy to understand what the function is doing by just reading its name. Likewise, simple operations just state what they are doing without the “Check” at the end like this:

def increaseQuality(item):
     return item.quality + 1

All the extracted functions can be now put in a separate file I called logic_checks.py and I added the from logic_checks import * at the top, so the functions could be used in gilded_rose.py and later in processing.py

While at it, I also extracted a few macros at the top of the file so it would be easy to modify these values in the code if requirements change.

Step 2. Extracting and unifying logical checks

In gilded_rose_ifs_extracted_unified.py I dealt with nested if statements. 

Usually, if-statements are used only to make forks in logic but in our case, there are “stairs” like these:

if itemNotBrieCheck(item) and itemNotBackstagePassCheck(item):
    if qualityPositiveCheck(item):
        if itemNotSulfurasCheck(item):
            item.quality = decreaseQuality(item)

These can be merged into a single condition with an “and”:

if itemNotBrieCheck(item) and itemNotBackstagePassCheck(item) and qualityPositiveCheck(item) and itemNotSulfurasCheck(item):
    item.quality = decreaseQuality(item)

It looks bad but we’ll fix this by wrapping the item checks in one check named itemNotSpecialCheck(item)

Also, we had some spaghetti logic in this block:

if sellInNegativeCheck(item):
    if itemNotBrieCheck(item):
        if itemNotBackstagePassCheck(item):
            if qualityPositiveCheck(item):
                if itemNotSulfurasCheck(item):
                    item.quality = decreaseQuality(item)
        else:
                    item.quality = zeroQuality(item)
    else:
           if qualityLessThanMaxQuality(item):
               item.quality = increaseQuality(item)

This block behaves like a “filter” with each else dealing with a product type. To separate each level, you can merge the if-statement with the one above it and invert the logic test to get rid of the if-else structure.

Here’s the result of one such step:

if sellInNegativeCheck(item) and itemIsBrieCheck(item):
    if qualityLessThanMaxQuality(item):
        item.quality = increaseQuality(item)

if sellInNegativeCheck(item):
    if itemNotBackstagePassCheck(item):
        if qualityPositiveCheck(item):
            if itemNotSulfurasCheck(item):
                item.quality = decreaseQuality(item)
    else:
            item.quality = zeroQuality(item)

As you can see, itemNotBrieCheck(item) is now itemIsBrieCheck(item) and is coupled with the higher level sellInNegativeCheck(item) check into a single independent if-statement. We don’t need an “else” anymore.

Step 3. Combining related checks

In gilded_rose_ifs_combined.py I simply highlighted the same “itemIs…” checks to spot blocks that can be combined into one and put the item type check at the top of the if-checks for all the items.

For example:

if itemIsBackstagePassCheck(item):
    if sellInLessThan11Check(item):
        item.quality = increaseQualityIfLessThanMaxQuality(item)
    if sellInLessThan6Check(item):
        item.quality = increaseQualityIfLessThanMaxQuality(item)

And

if sellInNegativeCheck(item) and itemIsBackstagePassCheck(item):
    item.quality = zeroQuality(item)

Becomes

if itemIsBackstagePassCheck(item):
    if sellInLessThan11Check(item):
        item.quality = increaseQualityIfLessThanMaxQuality(item)
    if sellInLessThan6Check(item):
        item.quality = increaseQualityIfLessThanMaxQuality(item)
    if sellInNegativeCheck(item):
        item.quality = zeroQuality(item)

Step 4. Extracting the processing for each block

Now that all the operations for each item are separated from each over, we can simply extract them into separate functions to make the result even more readable.

def update_item(item):
     process_standard(item)

     if itemIsConjuredCheck(item):
         item.quality = process_conjured(item)

     if itemIsBackstagePassCheck(item):
         item.quality = process_backstagepasses(item)

     if itemIsBrieCheck(item):
         item.quality = process_brie(item)

     if itemNotSulfurasCheck(item):
         item.sell_in = decreaseSellIn(item)

I separated the resulting functions in processing.py and imported them with from processing import *

Step 5. Commenting

Finally, we haven’t changed the logic behind the code. We did the most superficial refactoring (also the fastest). The code is just rearranged, keeping its messy logic behind it but now its much easier to tell what each block of code does and we can comment it properly for future reference.

Conclusion

As you can see, I have barely touched the code flow and logic. We merely rearranged it. Frankly, with these methods, you don’t need to fully understand what the previous developer has done. Of course, after every step, I ran all the tests to make sure nothing was broken but there’s no need to get into the nitty and gritty of the code. 

Don’t forget that our task was just to add functionality. Generally, as long as the code passes all the tests it is not our job to remake the horrendous work of others. It may not be at all the most elegant solution but the reality is that you will rarely have the time to do state-of-art refactoring when there are usually many more urgent matters and deadlines.

Anyhow, this successfully concludes the refactoring of the Gilded Rose code kata. Hope you found this practical guide useful and are now more motivated to write tests and refactor your code.

Happy coding and a good day!