Code Quality Matters

Over the last year as an engineer, I’ve had the pleasure of working on two teams which had its own set of experiences. Team A’s project is to extract a microservice from a monolithic code base and push to production, and Team B’s an inherited code base running in production and growing.

The experience with both these teams have been very different, apart from the tasks involved, the code quality between the two projects were on different levels. This article will be an excerpt of my experience with code quality, why it matters and how to tackle poor code quality without taking on a large amount of technical debt.

While I was interviewing for my first job right off college, one of the tasks I was assigned was to read a book called Beautiful Code by Andy Oram and Greg Wilson, since then I have lived by one quote when I develop.

Beautiful code is like a beautiful car. You rarely need to open it up to look at its mechanics. Rather, you enjoy it from the outside and trust that it will drive you where you want to go

— Adam Kolawa, Chapter 15, Beautiful Code

Expanding on this quote, we can define what poor code quality means, a beautiful car to look at, however with constant oil leaks on the way to the finish line. A few of the challenges we faced as a team included, code duplication, insufficient unit/integration tests, exception handling and logging, and releasing new features or bug fixes producing more bugs or breaking logic.

Code Duplication

The codebase is architected as Functions as a Service (FaaS), with each function as an AWS Lambda. Approximately 30 functions made up a full microservice, which led to duplicated code throughout the codebase since a majority of the functions performed tasks like sending a message to a queue or a topic. This was inconsistent throughout the codebase, and resulted in 3 different ways a message could be sent.

This also made it hard when making a small change to a function, changes would have to be made 2–3 different other functions to ensure the logic would not break.

Insufficient Testing

Tests that are beautiful for their simplicity. Tests that are beautiful because they reveal ways to make code more elegant, maintainable, and testable. Tests that are beautiful for their breadth and depth.

— Alberto Savoia, Chapter Seven, Beautiful Code

Testing acts as an implicit form of documentation, since each test case is a scenario of what each of your functions are supposed to do. Despite there being multiple test cases written, a majority of them didn’t work as expected or poorly written and hard to follow. This not only made the code hard to understand but also hard to work on small bugs.

Exception Handling and Logging

There was next to none handling of exceptions when we started on the project, this would result in false positives, assuming a service is working 100% of the time, but only to realise the exception was not being caught and not logging any of the exceptions. In fact while adding more exception handling, we broke critical logic.

Logging is important, and more important when your service is split over numerous functions. This can make it hard to follow where the code might be breaking, in fact during the initial stages of debugging an issue, I would have a printed version of the architecture so I could reference it.

There was a decent amount of logging for lower stages but not enough in production, it was also inconsistent which made searching for logs a lot harder.

New Features == More Bugs

One of the pain points has been when working on new features, it ended up creating more bugs in downstream services. Since the bugs would persist in downstream services it would take up developers time fixing bugs vs working on the feature.

Adapting and morphing code quality

In order to adapt and improve the quality of the code while maintaining feature deadlines. We had a few propositions on the table when tackling the problem.

Refactor code as main priority over feature work.
Refactor code in a language familiar to all and move to a monolithic codebase.
Refactor alongside working on features while taking on technical debt.

In the end we opted for taking on technical debt approach, the intention was to ensure that we could still meet feature deadlines as well as improve our code.

The solution was simple, a features user story had the following acceptance criteria appended.

Improve code readability where necessary.
Rewrite test cases in an agreed upon format.
Add logging and exception handling.

If refactoring was beginning to scope creep, it would be noted and added as an engineering debt. Although we haven’t reached the pinnacle of beautiful code, and well written tests, we have able to take on the debt, and improve the code base without impacting users*.

Debt is not simple to consolidate

Despite taking a measured approach when tackling the problem, we faced numerous challenges throughout, examples like adding exception handling, or refactoring methods into simpler forms. In once such case refactoring the following code had affected our analytics.

### OLD CODE
def get_test_type(self, is_ipv6):
    if is_ipv6:
        return "ipv4-prefer"
    else:
        return "ipv6-prefer"
### NEW CODE
test_type = "ipv6-prefer" if is_ipv6 else "ipv4-prefer"

The refactor was done based on the context of the boolean value, if is_ipv6 == True then it would return ipv6-prefer . When the analytics team noticed that there were no more tags with ipv6 we began digging deep into our code base, eventually we found out the old code was set in place as a solution for the following issue.

some_dict = { 'some_key' : '--' }
### OLD CODE
if some_dict['some_key'] is '--':
    is_ipv6 = True
else:
    is_ipv6 = False
print(is_ipv6)

False

The logic above would always default to False mixed in with the refactor, since is_ipv6 was False it would always return ipv4-prefer .

We were able to fix the problem by changing the above code as follows

some_dict = { 'some_key' : '--' }
### NEW CODE
if some_dict['some_key'] == '--':
    is_ipv6 = True
else:
    is_ipv6 = False
print(is_ipv6)

True

At the end of the day while working on this project I’ve learnt about the reasons why code quality is important. Improving the code quality, has allowed us to have better testing, catch errors in lower environments due to better logging and exception handling, set a standard for what new features should follow, and also reduce developers time working on features.

Mohsiur's Blog