Improving Quality - Finding Better Tooling + Extact Method

Summary¶

In my last article, I talked about taking a bit of a break and focus on refactoring for a couple of weeks. Specifically, I talked about the things that I was looking for with respect to more tools to help with the refactoring. This week, I talk about the progress that I made with the refactoring and my experience with the three added tools.

Introduction¶

Professionally I am an SDET, otherwise known as a Software Development Engineer in Test. When I mention my job title, people often think I am in a company to test things and break them. The reality could not be more different. Most of what I do is more properly defined as risk management and risk prevention. A large part of that is working to find better processes and to find better measurements for the quality of the projects that I work on. If I do things properly, I simply present the information to both teams and their management, helping them make smart information-based decisions that are keyed to the specific situation that they are in.

From that viewpoint, what I have been doing for the PyMarkdown project is not much different than my day job. There are many viewpoints that show risk in any given project, and it is a tough job to figure out which of those viewpoints to focus on at any given time. But even giving myself time to work on the compositional quality of the PyMarkdown project, I wondered if my usual tools (PyLint and Flake8) were good enough or if I can do better. So, as part of my work in the last two weeks, I picked out three new tools to experiment with, to see if they add any value to my well-established tool set.

What Is the Audience for This Article?¶

While detailed more eloquently in this article, my goal for this technical article is to focus on the reasoning behind my solutions, rather than the solutions themselves. For a full record of the solutions presented in this article, please consult the commits that occurred between 07 Nov 2021 and 20 Nov 2021.

Setting The Playing Field¶

As I mentioned in the last article, my usual choice of tools for analyzing Python projects is the combination of the Flake8 tool followed by the PyLint tool. While they do not have information to help with every refactoring or guideline that I want to use with Python projects, they do a respectable job in getting me most of the way there. The question I had asked myself was whether there were automation tools out there that could help me to boost the quality even higher than it is.

To that extent, I picked three different tools to try out: Code Inspector, CodeBeat, and Sourcery. The minimum qualification for me to use them was that they needed to have some integration with GitHub that activates when something is pushed to the repository. While two of the tools also offer VSCode support, it was not a requirement. After all, I run PyLint and Flake8 on the command line and as part of a lint job using GitHub Actions.

So on to the tools.

Code Inspector/Codiga¶

Before I started the refactoring last week, I was using a tool called Code Inspector, but today I am using the same tool, but now called Codiga. Some of the URLs for the tool still point to code-inspector.com, but I expect that they will be moved over to codiga.io before too long. And, as far as I can tell, the change has not affected the usefulness of the tool.

As I mentioned last week, my first impression of this tool was that it would be useful in helping me keep the number of new PyLint errors down. Essentially, as their tools page states, they use specific tools on their platform to scan the projects. You can opt in or opt out of scanning your project with any of those tools, so it is very flexible. But to be clear, they do not seem to have any custom analysis tools of their own, only open-source software.

But there is where they shine. Having used this tool for two weeks, I can positively say that I am going to continue using this tool going forwards. Whenever I make a change to source code in my VSCode editor, the Problems window at the bottom updates with any Code Inspector information within 10 seconds of me saving the file. By having the output in the Problems window with a line number and column number, I can click on it and go there to fix the issue. Since Code Inspector is running PyLint as one of its tools, that means I can find and address any PyLint errors without leaving the editor. That is cool!

In addition, their dashboard view of a project is a useful way to see what is happening with the project. The dashboard view for the PyMarkdown project is here and presents some nice graphs with clickable elements throughout. During the last two weeks, the dashboard and the views were immensely helpful in keeping me targeted on the most impactful changes. And when I want to improve the quality of a given code base, that is exactly the information that I need.

Evaluation¶

While this can be considered a repeat of the PyLint and Flake8 tooling, the in-editor experience and the reporting of those tools supplies enough of a value-add that I am going to keep on using this.

CodeBeat¶

Initially I was optimistic about the CodeBeat tool, as it looked promising. But after using it for the last two weeks, I am left in a confused state about what help this tool provides.

When using this tool, it seems to take a while to get my project scanned. Since I am not paying for this tool, I am okay with that. But when I do get a report, the metrics really confuse me.

One of their metrics, Function too long, is based on the actual number of lines in the source file, not the number of statements. As such, after the Black formatter has cleaned up my files and placed various function arguments on their own lines, I find that their line limit of 30 is breached often. I get that they are trying to keep each function on one viewable page within an IDE, but that metric does not sit right with me.

Then there is the Assignment Branch Condition too high metric. Brushing up on the available metrics, I found some valuable information on how to calculate this metric for Python. But no matter which function I try it with, the number that I get is always lower than the one they report in their errors view. And since they only list the final ABC value, I am unable to check my actual numbers against their actual numbers.

Finally, from my point of view, the Block Nesting too deep metric is broken. This metric is supposed to reflect the maximum number of block nesting levels in each function. That is not my research, that is their documentation. But unless my math is off, the depth level that they supply is the total block nesting depth, not the maximum block nesting depth. A good example of this is the following function from the Rule Md027 module:

def __handle_within_block_quotes_blocks(
    self, token, context, num_container_tokens, is_directly_within_block_quote
):
    if token.is_fenced_code_block:
        self.__handle_fenced_code_block(
            context, token, num_container_tokens, is_directly_within_block_quote
        )
    elif token.is_fenced_code_block_end:
        self.__handle_fenced_code_block_end(
            context, token, num_container_tokens, is_directly_within_block_quote
        )
    elif token.is_html_block or token.is_indented_code_block:
        self.__last_leaf_token = token
    elif token.is_html_block_end or token.is_indented_code_block_end:
        self.__last_leaf_token = None

The block nesting depth reported for this function is 4. I calculate it as a depth of 1.

Evaluation¶

While the idea of this tool was nice, I have removed it from the PyMarkdown project.

Sourcery.Ai¶

While I had an early idea that Sourcery would be useful, it was not until I started using it for refactoring that I was able to find out how useful it was. From the usability point of view, it was always spot on with what it found and suggestions on how to make it better. Looking at the list of possible refactorings, I believe that I have hit about maybe 25% of the refactorings presented.

Some of those refactorings were easy ones that I had missed. Others were making the code more readable by doing things like applying the De Morgan Identity to a conditional expression. Now, I have been doing that refactoring for years, but I always must write it out and manually work through the arguments of the expression, just to make sure that I get it right. With Sourcery, I just clicked on the apply refactoring menu item, and it was one for me. It was the same thing with applying their “Convert to Enumerate” rule. I could easily do it for myself, but I was always concerned that I was going to fat-finger something and break the code. This was a lot easier and a lot less error prone.

Those features alone would have sold me on using this as a tool. And then I ran into a couple of refactorings such as “List comprehension”. To be totally open, I struggle with comprehensions. I have tried three times to make some serious pushes into learning comprehensions, but I have struggled each time. I think that the List Comprehension might be the thing that helps me make progress on learning comprehensions.

Why do I say that? Well, there is the obvious fact that Sourcery detected a series of statements that can more compactly and efficiently represented using a comprehension. I know I sometimes have issues with just that. Then there is the ability to translate it with a press of a button. For me, that is a teaching moment. Being able to see that something can be changed, and having that change being applied gets through to me more than any amount of reading can do. And it works even better if it is something I am familiar with and not a made-up example. I think it is safe to say, it just hits home in a new way for me.

Evaluation¶

Basically, if I had to assign a role to Sourcery and how it helped me with the PyMarkdown project, it would be role of mentor. It was not the role I assign to PyLint and Flake8, which is essentially a monitor role. Besides, that role is solidly filled by the existing tools PyLint and Flake8. Sourcery provides me with better ways to do things, providing me with solid information on how things would look after that change, how to make that change, and why it should be changed.

And as I can always use a good nudge in the direction of cleaner code, this is a tool that I want to keep in my toolbox.

Oh, and did I mention this other nifty feature? If I forget to apply a suggested refactoring, Sourcery creates a new PR with their suggested changes already applied, ready to merge into a newly committed branch at my command. While I am not usually forgetful when it comes to process, it is nice to know that Sourcery has my back!

Choosing a Quality Measurement¶

Having experimented with the Sourcery’s Quality measure for a function, I was pleased that its measurement of quality was mostly coordinated with my own observations. To be clear from the start, I do not have any plans to stipulate that all of the functions in the PyMarkdown project to have a Quality measure in the 70s or higher. While it might be nice to get there on smaller projects, I do not believe that achieving that measurement on larger projects such as PyMarkdown are achievable. And even if they are achievable, I believe that the breakdown of the functions to achieve that measure would trash the readability of each function.

But looking through their reports, I had to come up with a quality score that I at least thought was a good target for an initial push to clean up the project. I found the one boundary of 25 to be too low and needed all the functions in the project to be higher than that. Experimenting with a couple of functions and what their Quality score were, I decided that a score of 40 was a good starting point. Why 40? I just found it to be a satisfactory level of quality without being too disruptive to the readability of the project.

Refactor: Extract Method¶

Having picked a minimum quality score of 40 for each function, I started working my way through the PyMarkdown project, one module at a time. The process of going through the entire project was long and arduous, taking the better part of two complete weeks. The process was also very monotonous, as it was always the same thing:

1. Check through the next module, examining the reported Quality for each function.
2. Find the next lowest code quality function in the current module.
3. If the quality for that function is 40 or above, go to Step 1.
4. Otherwise, examine the function and Find a good block of code to extract.
5. Add a marker function call with the name of the new function.
6. Create a new function and extract the code to that new function.
7. Add arguments to the new function to satisfy any missing variables from the function.
8. Copy that argument list and add it to the marker function’s argument list.
9. Go through all assignments from the function and add them to the list of variables to be returned by the function.
10. Copy that return variable list to the marker function.
11. Use VSCode to figure out which variables are not used by the original function and remove those variables from both lists.
12. Run the full set of PyTest tests, looking for any asserts that show something is wrong.
13. If something is wrong, fix it. If it cannot be fixed, revert the file and start over.
    • The most likely thing that happened to me was that a variable was not assigned before usage. This happened with variables that were assigned in optional code blocks. The fix was to make sure to add default values before the optional code block to ensure that variable was set to something reasonable.
    • In some cases, I entered the wrong code, and I just got either a syntax error or a test failure. In those cases, reverting the file was the easiest way to go.
14. Verify that it looks right, and stage that change to the repository, so it does not get lost.
15. Look at the new quality score for the function. If it is below 40%, go to Step 4. Otherwise go to Step 3.

As far as the PyMarkdown project was concerned, that was my life for two weeks. It was not overly exciting, but it was rewarding. Even as I was making my way through the modules, I could see that the readability of the modules was already increasing. I could be wrong, but I believe there is only one place in the refactored code where I simply added a number to the end of an extracted method. In all other cases, I was able to find a solid, logically place for the function to be split up.

Cleaning Up The TODO Items¶

One thing that Codiga listed in its Violations view were the number of # TODO comments that I had littered around the PyMarkdown project code base. While I did not initially agree with their definition of these comments as violations, I started to come around quickly. Deciding to get them out of the way, I spent Saturday afternoon going through most of the TODO comments. In each case, I either removed the comment, fixed the issue and removed the comment, or logged a new issue and removed the comment. When all was said and done, the comments for Issue 145 detailed each of the TODO comments and what was done with it.

To be honest, there were an equal amount of “what the F***?” comments as there were “didn’t I already…?” comments. Then there were the “yeah, I should have done this long ago” comments.

Basically, it was good to just get these things done, figured out, or removed from the code base.

What Was My Experience So Far?¶

The reason that I decided to write a linter in Python is that I wanted to learn Python and this was an excellent project to do that with. The reason I chose a Markdown linter is because I did not feel there was a good, flexible linter for Markdown out there. Specifically, I picked a linter because linters help developers look within their content to see if there is something that they can do to be more consistent and to raise the quality of their content.

And that is what I like about the PyMarkdown project. I am still learning about Python and I know I have a solid set of rules that can help Markdown document authors. Additionally, I know I can write more rules to help those same authors apply extra structure to their documents.

That is why, even through the boring refactoring effort, I kept on going. Because I knew that this work was helping me to help myself, and to help others. As altruistic as it sounds, that is enough for me.

What is Next?¶

I still have a couple of smaller items to clean up, but the big refactoring work has been completed. I do want to make more progress on bugs, so I will probably split my time between those two efforts. Stay tuned!

Like this post? Share on: Twitter ❄ Facebook ❄ Email