Refactoring: From quick and dirty to pythonic I - Sun, Apr 10, 2022
Refactoring: From quick and dirty to pythonic I
Refactoring: From quick and dirty to pythonic (Part I)
In an effort to more “shift left”
tests and checks in a GitOps
deployment pipeline, I needed a script that would use the information contained in Flux HelmRelease manifests
, Flux GitRepository manifests
and k8s ConfigMaps
in order to use the helm template
command to render the k8s resources and check these against some kyverno cluster policies
That would enable me to get possible policy violations much earlier in a CI/CD cycle.
So I wrote a python script that does exactly that. The code of the first working version of this script can be found here . I have to admit that time pressure as well as a seeing CI/CD code as not needing the same kind of quality as application code, resulted in severe sings of quick & dirty work.
So after getting this first version to work in the deployment pipeline, I decided to take a little break, look at the code from a higher perspective and lay out a plan how to refactor * it so that it would look like a real result of software engineering craftsmanship .
This, and the following blog post describe the steps I took in order to achieve this goal.
* Please note that I used the first edition of the Refactoring book. For some more smells and refactorings please see refactoring.com which is an excellent source for refactoring.
Analyzing the code
Looking at the scripts code some smells of bad code become obvious pretty quick:
- Long methods with Spaghetti code
- Nested loops and if statements
- Copied and pasted code
- Somewhat strange mixture of oo-style programming and functional programming
Linting the code also revealed some problematic parts but surprisingly gave it
8.21/10, indicating again that perfectly linted code is not necessarily good code.
So I started a broader systematic analysis by going through the individual code smells Martin Fowler describes in his book Refactoring . Here ist the list of code smells I found applicable for the code and my evaluation of those smells:
- Duplicate code. In more than one place. The obvious refactoring to apply would be Extract method which is part of most IDEs anyway.
- Long method. Applies to almost all the functions. Refactorings most applicable to solve these smells are Extract method , Decompose Conditional (especially true for the nested ifs) and Replace Method with Method Object (caution: Applying this refactoring might in fact increase the oo/functional programming mashup)
- Long parameter list. Three parameters at most, so not really a problem
- Data class. Since I specifically added data classes this code smell -more related to oo-style programming- is applicable but needs to be weight against the intention to deliberately separate data classes from functions.
Identified Refactorings and patterns to start with
The most obvious refactorings to apply are Extract method and Decompose Conditional as their application would increase the codes readability and likewise decrease its complexity. Looking at the main responsibilities of the code: Parsing yaml and creating the corresponding objects, using one of the Creational software patterns seemed worth examining thus applying the refactoring Replace Method with Method Object .
Although the code does what it is supposed to do and has some decent linting score, it has severe shortcomings that hinder further development and decrease its maintainability.
My analysis of the code identified three Refactorings / patterns that should be applied to the code:
- Extract method
- Decompose Conditional
- Introduction of the Builder pattern
The application of these patterns should yield the following benefits:
- Easier to read and comprehend code
- Better code maintenance
The main drawback might be that more source code gets written that is not necessarily related to to the responsibility of the script.
If you are interested how the script evolved, I invite you to read the second part of this blog post series.