Refactoring

Refactoring is the process of improving code without altering behavior. For instance, refactoring a function would not alter its inputs, outputs, or side effects, but would alter the code internal to the function. On the other hand, refactoring a feature would not alter the behavior of a feature, but may significantly alter various functions, including adding new ones and removing unnecessary ones.

Refactoring can be used to either improve code that has been published and is active, or to improve code during development before it goes into production.

Refactoring code during development is important. The development process starts with the design phase. This involves using research and experimentation to explore the problem space, and come up with a potential solution. Time spent during the design phase is subject to diminishing returns; if everything is completely correct with the initial software implementation, too much time was spent on design.

Instead, it is more efficient to get the initial design correct with regards to major decisions, and then discover additional minor problems during the first implementation. It is important not to stop after the first implementation; instead, as problems are identified, the code should be iteratively refactored and improved, until both the code and the design are robust. This decreases the time to develop an application or feature, and the software produced tends to be more reliable.

Once code has been published, requirements will change, and the quality of the codebase may drift. To help prevent that drift and maintain low MTC, the code will periodically need to be refactored. We have discussed being “wrong in correctable ways” in previous chapters, and if the code was written according to the rules in this book, the refactoring process should be relatively painless. However, it is important to recognize we may not have the luxury of code that is simple to understand and easy to change. Often, we are working with software written many years ago, and it feels like it has been cobbled together with little regard for the sanity of the maintainers. In such cases, we have received a message in a bottle, but it has degraded during its journey.

This chapter will cover how to apply the rules in this book to code that does not adhere to those rules. It will go over the proper process to reduce the chance of introducing bugs, what problems to look for, and how to recognize if a refactoring attempt should be abandoned. Refactoring is a deep topic, and this chapter is only meant as an introduction to it. I encourage the reader to explore refactoring in greater detail both by reading books dedicated to it and through practice.

Before You Begin

There are two common reasons programmers do not refactor code, even when small improvements could have large, positive impacts: time constraints, and fear. Time constraints require frank discussions with management about the relative value add and tradeoffs involved. It is not uncommon to find yourself in a situation where the organization values feature development over foundational improvements. Navigating those circumstances is challenging. I advise approaching all such interactions with honesty, humility, and copious data.

Fear is different. It tends to arise because the process of changing code involves breaking it, and breaking code results in compiler errors, failing tests, and potentially introducing bugs. A healthy dose of fear is not a bad thing; as long as it does not paralyze us, it can often result in making fewer mistakes as we approach improving code.

When refactoring, it is key to adhere to the following guidelines:

• Use version control.
• Edit code in a development environment, never change code directly in production.
• Use unit tests to ensure existing behavior remains consistent.
• Avoid automation that transfers code from development to production without human review.
• Copy production data (or generate new data) for use in your development environment.
• Ensure all external dependencies are configured for development. That is, connect to a development database, use external development APIs, etc. While this is not always necessary, err on the side of caution, and be mindful of using any production system during development.

If you have a fear of refactoring, you are not alone. The best way to break that fear is to break the code, demonstrating the worst that can happen is not so bad (assuming that you follow the above practices). Modern tooling allows us to easily revert changes, so there is no permanent damage if it does not work out. Additionally, keep refactors targeted and focus on small, incremental changes to achieve an overall improvement. This way, if a misstep is taken, the impact is minimal.

Know the Tools First

Every language is different. They come with different package management systems, different formatting conventions, different standard libraries. Within the definition of Mean Time to Comprehension (MTC) provided in the book’s Introduction, there is an qualification: “a programmer familiar with the given … libraries, tooling.” - i.e., we assume that whoever is reading our code is at least familiar with all of the essentials around that code. That familiarity is critical: without it, you cannot expect to understand the code you are working on.

Before refactoring, make sure you understand the language, including its standard library and all the major libraries or frameworks the project depends on. Read about them. While going through the process of refactoring, if you do not understand some facet of the language, stop and look it up. Perhaps implement a tiny program to familiarize yourself with how that element is used.

Ensuring you know the tools can forestall awkward refactor attempts that are the result of unfamiliarity with software using that particular language and that particular framework.

Identifying Problems

The actual process of refactoring code will depend on the language and structure, but there are some common approaches that can help. The first step in refactoring is identifying problems with the current code. The rules covered in this book provide guidance to do this:

• Nested control blocks, such as for loops, if statements, switch statements, try|catch blocks, etc. (chapter 2, “Recipe”)
• Nested function calls, like: A(B(C(D(foo)))) (chapter 2, “Recipe”)
• Unnecessary variable mutation (chapter 3, “State”)
• Global variables (chapter 3, “State”)
• Mixing building logic with business logic (chapter 4, “New”)
• Conditional logic that could be modeled using the type system (chapter 6, “If”)
• Shorthand or meaningless variables names: a, m1 , myVar, etc (chapter 7, “Naming”)
• Variables that seem to be re-used for multiple purposes (chapter 7, “Naming)
• Inconsistent style or naming (chapter 7, “Naming)
• Excessive in-line comments, explaining what a line of code does, instead of why the line is necessary (chapter 8, “Documentation”)
• Lack of documentation about expected inputs and outputs (chapter 8, “Documentation”)

Simple Improvements

Let us examine the following function, which calculates the standard deviation of an array of numbers:

# We're going to avoid most built-in math functions,
# to make the code more interesting
import math

def StdDev(array):
    Sum = 0
    Total = 0
    n = len(array)
    for i in range(len(array)):
        n = array[i]
        Sum = Sum + n
    if n < 2:
        raise ValueError("Must have at least 2 numbers in array")
    mean = Sum / n
    for i in range(len(array)):
        Sum += (mean - array[i]) * (mean - array[i])
    Total = Sum / n
    Total = math.sqrt(Total)
    return Total

Most programmers will have the instinct that something is amiss with this code. Using the guidance of the rules outlined above, we can concretely identify the issues. I encourage you to take a moment and identify any problems you see.

Recall it is important to create unit tests to ensure behavior remains consistent across refactors (see chapter 9, “Unit Testing”).

This simple unit test helps us ensure consistent behavior:

import unittest
from . import math_utils

class TestModule(unittest.TestCase):

    def test_stddev_invalid(self):
        self.assertRaises(ValueError, math_utils.StdDev, [])
        self.assertRaises(ValueError, math_utils.StdDev, [0])

    def test_stddev(self):
        self.assertEqual(math_utils.StdDev([1, 2]), 0.5)
        self.assertEqual(math_utils.StdDev([0,1,2,3,4,5,6,7,8,9]), 2.872281323269)
        self.assertEqual(math_utils.StdDev([1, 4, 5, 7]), 2.165063509461097)
        self.assertEqual(math_utils.StdDev([344, 1002, 45,222]), 361.66515936706)

We call the function we want to verify, and check the results against our expectations. If the behavior of the function changes, the test will alert us to this fact by failing. There is always some risk here: in a system with enough complexity, ensuring all behavior remains consistent can be quite difficult. This is why refactoring should be done in as many small, incremental doses as possible, since doing so allows us to more easily validate that the changes did not impact behavior.

Let us take a look at a rewritten version. Try to identify some of the changes we made, and whether they were the ones you expected:

# We're going to avoid most built-in math functions,
# to make the code more interesting
import math

def stddev(numbers):
    """
    Given a sequence of numbers, calculates their standard deviation.

    Args:
        numbers (Sequence[Union[int, float]]): A sequence of numbers.

    Returns:
        float: The standard deviation of the sequence.
    """
    if len(numbers) < 2:
        raise ValueError("Must have at least 2 numbers in array")

    num_sum = 0
    for number in numbers:
        num_sum += number

    mean = num_sum / len(numbers)

    deviation_sum = 0
    for number in numbers:
        deviation = mean - number
        deviation_sum += deviation * deviation

    variance = deviation_sum / len(numbers)
    return math.sqrt(variance)

StdDev = stddev

In the above example, we made several improvements to reduce MTC:

• There were intermixed variable naming conventions. We updated the code to adhere to PEP8, the formatting standard for the Python language. This is not by itself enough to make code readable, but consistency is a step in the right direction (see chapter 7, “Naming”). Because we are altering the behavior of all code in this function, the style changes do not have to be done separately.
• We changed the variable names to be more meaningful. Instead of using generic names, we use the mathematical terms for the individual elements used in the calculation (see chapter 7, “Naming”).
• The original code was not taking advantage of Python’s for loop to easily to iterate over a list of items, instead accessing using the index. We could have used reduce or some other construct, but in Python a simple for loop is faster for most programmers to understand (Know The Tools First).
• We eliminated some unnecessary variables and statements, but also added another variable (deviation) to help clarify things and improve flow (see chapter 2, “Recipe”).
• We moved the value check to the beginning of the function, for clarity and to prevent unnecessary execution (see chapter 2, “Recipe”).
• We added a doc block to clarify the function’s purpose and usage (see chapter 8, “Documentation”).

Note that we aliased the old function name at the bottom of the example, despite its non-compliance with PEP8 coding standards. This is because removing it entirely would break the existing API of the function, and would potentially propagate changes to the rest of the codebase, or even other software systems if this function is a public part of a package meant for other applications. In general, when we want to change a function name, we must evaluate if we should alias the name to keep backwards compatibility (if the function is used outside this codebase, this would be the correct approach) or simply update the name and all the references to it (if the function is entirely internal, we could do this).

Law of Demeter

The Law of Demeter (LoD), also known as the Principle of Least Knowledge, states that code should only access its immediate dependencies, and should avoid reaching into those dependencies to obtain access to additional references. If those references are needed, they should be added as dependencies in the form of arguments or fields. Often the process of refactoring to follow the LoD eliminates unnecessary code, sometimes even entire functions or classes, which only existed in the first place to cover up the fact that the LoD was being broken. Following LoD improves the clarity of the code, and as a result, reduces MTC.

Here is an example of breaking the LoD:

def build(obj):
    obj_id = obj.value.id_generator.generate_id()

    return BuiltObject(obj_id, obj.defaults, obj.prop.overrides)

This violation of the LoD causes several problems:

• If the internal structure of the passed obj changes, this code will break, indicating it is fragile. Coupling has been accidentally introduced, and the maintainer of the underlying id_generator class (or the underlying value or props class) may have no idea that the build function relies on them.
• Interacting with this function is onerous, as an entire obj must be created somehow in order to call build.
• This function lacks clarity, because the real requirements are hidden within the function’s code, where properties from obj are accessed.

Rather than reaching deeply into the obj variable, build should request what it is using directly, as arguments. We therefore refactor the code as such:

def build(id_generator, defaults, overrides):
    return BuiltObj(id_generator.generate_id(), defaults, overrides)

We can see that obj as a dependency was eliminated entirely here. Instead, build only requires the exact values it needs in order to accomplish its task.

Following The Specification

A common difficulty when refactoring is determining whether or not the existing code is actually correct. This is because correctness of code cannot be evaluated simply by looking at its behavior in isolation. We must know the specification for the code, which dictates what “correct” means. Specifications are often poorly documented, or perhaps not documented at all. It is our job as programmers to research and understand the real requirements.

The following program builds a dictionary of keys and related values based on an input string containing those values.

Here is the string value: time=1998-10-10,distance=5554,color=blue

import re

def parse_values(values):
    result = {}
    result["distance"] = re.search(r"distance=([\w\-]+)", values).group(0)
    result["time"] = re.search(r"time=([\w\-]+)", values).group(0)
    result["color"] = re.search(r"color=([\w\-]+)", values).group(0)
    return result

One positive of this example is that the formatting already adheres to PEP8. However, there are still some problems with it.

A few things jump out immediately:

• Every time this function is called, we recompile the same regular expressions.
• The pattern of searching for a regex seems to be repeated.
• What happens if this fails to find a value? Currently, it will fail with some esoteric error message. Let us assume the spec says the value should not be set in the returned dictionary.

We can rewrite it to improve these things (for this example, we will assume there are unit tests in place):

import re

value_re = {
    "time": re.compile(r"time=([\w\-]+)"),
    "distance": re.compile(r"distance=([\w\-]+)"),
    "color": re.compile(r"color=([\w\-]+)"),
}

def parse_values(values: str):
    result = {}
    for k, r in value_re.items():
        v = r.search(values).group(0)
        if v:
            result[k] = v

    return result

This is better. By using a map (see chapter 6, “If”), we can easily add more values, we handle errors properly, we are not recompiling on every function call.

We may look at the code now and believe it to be correct: it behaves as before, is more performant, and has less duplication. However, recall that correctness is determined by the specification. To know with certainty that we have refactored the code correctly, we must refer to that specification.

In the case of the above example, let us imagine the specification for this data format is an arbitrary number of comma-delimited properties, where the keys and values are separated by an = sign. Perhaps when the code was originally written, the only properties were time, distance, and color, but that was coincidental. That coincidence, however, caused the original programmer to believe the code was correct, as it parsed all the available data.

Because this code does not adhere to the actual specification, the original behavioral model was wrong. This is a normal part of programming, and when we encounter these situations, we should be aware that the code likely provided value for some time, and that is important. However, when refactoring, we should make sure we know the actual specification, and refactor code to reflect that knowledge.

Start by updating the unit tests (refer to Contract Testing in chapter 9, “Unit Testing”).

import unittest
# This is the module with the parse_values function
from . import value_parsing 

class TestModule(unittest.TestCase):
    
    def test_parse_values(self):
        self.assertDictEqual(
            "time=1998-10-10,distance=5554,color=blue",
            {"time": "1998-10-10", "distance": "5554", "color": "blue"}
        )
        # Here we add a unit test to reflect 
        # our updated understanding of the spec
        self.assertDictEqual(
            "key1=value 1,key 2=value 2,KEY3=VALUE3",
            {"key1": "value 1", "key 2": "value 2", "KEY3": "VALUE3"}
        )

Then we can refactor the code to reflect our understanding of the spec, and pass the updated unit test.

def parse_values(values):
    """
    Given a string of key=value pairs, parse into a dictionary.

    Args:
        values (str): A string of key value pairs, in the form
            "<key1>=<value1>,<key2>=<value2>,etc"

    Returns:
        dict: A dictionary with the parsed keys and values.
    """

    result = {}
    kv_pairs = values.split(',')
    for pair in kv_pairs:
        k, v = pair.split('=', 1)
        result[k] = v
    return result

Or, even simpler and taking advantage of the available Python syntax:

# The downside of this approach is that, while it
# does the same thing as the previous example,
# it can be confusing to those less familiar with python
def parse_values(values):
    # (The doc block is left out of this example for space reasons)
    return { pair.split('=', 1) for pair in values.split(',') }

No regex, new properties do not need to be handled individually, and the code is concise. It appears to properly adhere to the specification.

There is a downside: if any of the keys or values use the reserved tokens , or = with escape characters introduced, a full tokenizer and parser will need to be used (note that most languages have some lexing functionality built in). Refactoring to address this issue is left as an exercise for the reader.

Abandoning a Refactor

Refactoring code is a wonderful way to familiarize ourselves with a codebase. Sometimes, we may start refactoring a project only to realize that it was implemented a certain way for a reason, and changing it is more complicated than we initially thought.

When this happens, remember that the time spent was not a waste. We gained a deeper understanding of both the code and the problems that it solves.

The knowledge gained from the experience is valuable, and will help us navigate the project moving forward. At some point in the future, we can return and improve the codebase in other ways that still respect the project’s requirements and constraints.

Do not view abandoning a refactor as a failure. Doing so can introduce a fear of failure that will make it more difficult to begin refactoring again in the future, and make it more difficult to stop refactoring when it becomes clear the code will be worse due to the changes. The knowledge gained from the process is always a success.

Ask yourself the questions: what documentation could have been present in the code that would have allowed me to understand why it is written the way it is? How can I communicate to someone in the future what I have learned during this process so that future refactor attempts are more productive?

Record the answers to those questions within the codebase, as either doc blocks or comments, and outside of the codebase in supporting documentation. This will ensure the hard-won knowledge you gained is shared with future maintainers and will make their job that much easier.

Conclusion

These examples really just scratch the surface. In large, complex projects, the process of unraveling a codebase may take weeks, months, or even years. As much as possible, the refactoring process should be approached in incremental steps, where each step brings the code to a more cohesive, simpler state.

The ability to refactor may be the most vital skill a programmer can have. It improves the ability to write new code, it improves existing code, it reduces the time others have to spend on understanding or working within a codebase, it decreases bugs, it deepens understanding of the codebase as a whole, and it frequently results in improvements to the end user experience.