Monday, May 9, 2022

Colby Peck - Gateware Week 5

I’m just now finishing up the fifth week of my time here on the Gateware team. I ended up accidentally joining a month earlier than I was supposed to (bit of a long story), so this is the end of my first week as a proper Gateware dev where I was able to dedicate my full attention to the project. This first post of mine is going to be a bit lengthy, since it covers 5 weeks of work; I hope you’ll forgive me that, dear reader.

My first task was to add some new functionality to the single header compiler; the ability to specify files to be prepended or appended to the single header. My onboarding process was a bit delayed, so I wasn’t able to make or commit any changes for my first few days on the project. I spent that time reading through the entire single-header compiler and writing down - in plain english - what all the code was doing in a separate document. This ended up being more useful than I first anticipated. When I was later fully onboarded, my first act was to refactor the code I was about to change - to make it more mutable - and I had just written a guide to the whole thing. All told, the refactor and feature addition itself only took about a day’s work once I sat down and really got to it, but it was only so straightforward because I fully comprehended the code before I began.


After the relatively painless work on the compiler, I was due for a task that would have me pulling my hair out at moments - such is only fair. The task in question was to suppress and fix the warnings generated when Gateware is compiled. A simple task to understand, but completing it took a lot out of me. Since Gateware is multi-platform, it’s internally compiled on Windows, Linux, and Mac as part of the automated testing pipeline. This meant that I had three different compilers to work with. Not a huge issue on its own; it’s not hard to find the appropriate preprocessor directives to suppress specific warnings. Then, all I had to do was make sure those preprocessor directives got parsed by the compiler before it compiles Gateware. Not hard for the single-header version: I just got all of the directives into some compiler-specific #ifdef blocks and used the new prepend functionality I had just added (as well as adding some blocks that unsuppressed the warnings to the end with the append functionality). But suppressing the warnings on our pipeline was a much trickier process - I’m feeling exhausted just recalling how much digging around it took. After several days of trying various configurations and placements of preprocessor directives (none of which worked), I figured out how to suppress warnings with the command-line arguments used in our cmake scripts. That did the trick.


Following the warning suppression, I was tasked with some work in the GMatrix library. LookAtLH needed a right-handed variant and some of the projection-matrix-construction functions had gotten mirrored versions, but hadn’t gotten their unit tests. Making the new function was fairly simple, but when I opened up the GMatrix unit tests, I found that all of the tests only tested a single case. So… In addition to adding the new tests, I expanded the coverage of nearly all of the existing ones. Every test got a refactor pass as well. 


I made a real-life transform gizmo out of LEGO to help visualize the various matrix functions and how they were supposed to behave. It was delightful and very helpful.


When I added some new cases for the RotationGlobalY test, I butted up against an edge case in Gateware’s floating-point comparison. Now, float comparison is a bit of a rabbit hole that I won’t take you down; suffice it to say that comparing floats can get messy. Anyways, I ended up refactoring Gateware’s float comparison macros.


Old comparison macros, now deprecated

New comparison macros



They're more-or-less in their final form now, pending discussion and review with Lari. We might need a macro that allows for a hybrid test with two different margins (one absolute, one relative).



Refactoring & Legacy


You may notice that in each of my tasks, I’ve refactored some legacy code. As I’ve gone through with the refactors, I’ve been thinking about the nature of refactoring code in the context of long-term group projects like Gateware. If you’d indulge me, I’d like to lay out my thoughts regarding the matter in the next few paragraphs.


The goal of refactoring is to make code more readable. But why is this important? Because in order to change code, you must comprehend it. The less readable the code, the harder it is to comprehend, and therefore the harder it is to change. Hard-to-read code is hard-to-change code; rigid code. And codebase rigidity makes change (and therefore progress) more difficult. So it’s vitally important for a project’s long-term health that the codebase be made as mutable as possible, and good readability is the chief factor of good mutability. If I know how something works, I can change it. If I have no idea what’s going on, trying to add changes will break things in unexpected ways.


However, refactoring legacy code feels kind of disrespectful. It feels like reorganizing someone else’s workspace when they aren’t looking. My discomfort has some roots in imposter syndrome: “Who am I to say that my factoring of this code is cleaner or better than the one that’s already here?” It feels somewhat judgmental too. “I, the arbiter of clean code, have judged thy code messy.” To address these feelings, let me tell you a story.


When I went home for Christmas last year, I ended up doing some work in my uncle’s wood shop. At one point, he had me use his belt sander (yay, power tools!) Now, this belt sander was a bit of a mess: it still had an old, tattered belt on it and the casing was full of sawdust. When I saw this, I didn’t think less of my uncle for it. I didn’t blame him for leaving his sander in a messy state at the end of a long workday. But I did disassemble and clean the sander before I used it, and I cleaned it again once I was finished.


An ideal codebase gets perpetually cleaner and easier to change. In order to make that happen, all that’s needed is for each programmer to leave whatever code they work on in a slightly cleaner state than they find it (or at least not to make it messier). Conversely, if we do not allow ourselves to clean up the code that we’re working on (via refactoring), the codebase can only get messier. So, “Who am I to say that my factoring is better than the one that’s already here?”  Well, I’m the guy working on this code. For the moment, this shared workspace is my responsibility, and I’ll keep it as tidy as I can. And as for “I, the arbiter of clean code, have judged thy code messy.” …That’s a very draconian way of putting it, but it contains some truth. In order to clean something, you must acknowledge that it’s messy. We must be honest with ourselves if we want to keep our codebase clean. The important distinction is that calling someone’s code messy isn’t equivalent to calling them a bad programmer. The natural state of code is a barely working mess. Any programmer with any amount of experience has learned this firsthand. I write messy code too! But when I am fortunate enough to have the time and energy to clean my code, I do. And when I work with legacy code that’s messy, I clean it. My goal with this practice is to leave everything I touch just a bit better than I found it - and hopefully make the lives of Gateware programmers (myself included) easier.


No comments:

Post a Comment