Copy
View this email in your browser
Arch-Engineer
Code better

The false promise of "Reading by Refactoring" and friends


What to do when a newcomer wants to learn your codebase? "Reading by Refactoring" is an appealing choice advocated by many: read through the code, and make it better as you go along. Similar ideas, less discussed, are "reading by testing" and "reading by documenting;" I'll call them collectively "reading by contributing." It's a nice promise: let a newcomer to the codebase do the valuable work of fixing up your code quality while also learning enough to do bigger things. I myself found the idea intuitively appealing, and told a number of people about it over the years.

I've now tried both reading by refactoring and reading by testing on two different projects, and read some more books on learning in the meantime. I've concluded: when you reflect on the knowledge and skills exercised in reading by contributing vs. actual software development, it becomes clear that reading by contributing is an appealing idea that just doesn't work, and doing them actually made me slower at learning codebases.

Let's look at an example. Consider the following code snippet, and think about how to refactor it. But, unlike my usual toy snippets, which are meant to be understandable without context, here I'm trying to illustrate the experience of reading-by-refactoring, and so I've picked a snippet from a real codebase — specifically my game mod Ironfist, which contains a mixture of new code and code from the original game. I've picked a snippet intended to be a little bit understandable with minimum context, but also with references to concepts specific to this codebase. I may have failed on the first criterion, so I'll give you a little context: this is code from the game's map editor which takes an "overlay," a game object consisting of an upto-6x8 grid of tiles, and places it on the game map. Specifically, it's part of the code for placing castles on the game map. If you're familiar with gaming, that should help a tiny bit.

So, here's the snippet. You can skim past it if you want. But if you actually pause and try to figure out how you might behave if you were to apply reading-by-refactoring to this code, then there's a teachable moment coming.

if (OverlayMaskBitSet(&ovr->coveredNonObstructedMask, j, i)) {
    if (tile->overlayIndex == -1) {
        tile->ovrLink = giCurOverlayIdx;
        tile->overlayIndex = ovr->fullGridIconIndices[ovrTileIdx];
        tile->overlayTileset = ovr->tileset;
         if (i < 4
              && OverlayMaskBitSet(&ovr->intersectsTileMask, j, i + 1)
              && (ovr->locationType != LOCATION_ROAD ||
                    !OverlayMaskBitSet(&ovr->interactionPointMask,
                                       j,
                                       i + 1))
              && !OverlayMaskBitSet(&ovr->shadowsMask, j, i + 1)) {
            
            tile->hasLateOverlay = 1;
        } else {
            tile->hasLateOverlay = 0;
        }
         if (OverlayMaskBitSet(&ovr->animatedLateOverlay, j, i)) {
            tile->hasOverlay = 1;
        } else {
            tile->hasOverlay = 0;
        }
    }
    //  rest of code omitted
} 

I admit being a little bit unfair with this example. I made a beeline to a certain 500-line function to find this snippet. It uses bit masks, because this code is literally from the 90's. Still, some of you are probably reading this and thinking "doesn't look so bad."

Let's think about some of the concepts and questions that need to be addressed to fully understand this code:
  • One needs to understand what the various bitmasks such as the "covered non-obstructed mask" and the "interaction-point mask" mean
  • One needs to understand the structure of a "tile," which itself requires understanding other concepts such as tilesets and "late overlays."
  • One needs to know what a road is in the game, and how they interact with castle-placement.
  • One needs to understand what that "i < 4" is doing there. (Answer: Hardcoded knowledge that all castle objects have a certain shape.)
Now, let's think about what knowledge you'd be gaining by applying any of the reading-by-contributing techniques.

For reading-by-documenting, well: this code is in the middle of the 500-line PlaceOverlay function, so most likely you'd document the function "places an overlay on the map" and call it a day. I know this codebase in detail, and I can't give much of a better description to this snippet than\ "does tile setup for placing a town overlay." You can figure that much out and still not address any of the questions above. Reading-by-documenting won't give you this understanding.

For reading-by-testing, the story is similar. You can test the function as a whole properly places towns. Or you could test that this snippet.....sets bits on the tile in the way that it does currently? It's very hard to read this and identify a more general property. Of course, the true testing aficionados would tell us this code first needs to be refactored to be more testable, which brings us to...

Reading-by-refactoring advocates name six specific refactorings to be exercised during this process:
  • Rename
  • Extract method/variable/field/parameter
  • Inline
And indeed, there are plenty of places where you can apply these transformations. You can rename i and j to row and col, or put this entire snippet into its own function.

But you might notice that, with the partial exception of Rename, all of these operations are very syntactic, like mechanically finding similarities. And so, again, reading-by-refactoring does not use, and therefore does not cause you to gain, the answers to the 4 questions above.

In summary, one can apply the reading-by-contributing techniques to satisfaction and yet still be unable to work on this code. In learning theory terms, there is little retrieval practice of the actual knowledge used in working on this codebase.

And yet this code is nonetheless greatly in need of refactoring.1 There is hardcoded knowledge about what should be configurable data, the tile structure is complicated and unintuitive, and the knowledge behind and intention of this code is generally expressed illegibly. But even noticing these opportunities requires already having this deeper understanding. If you paused earlier to attempt reading-by-refactoring on this code, how many of these occurred to you?

Now, imagine you were given any task elsewhere in the codebase that involved touching map tiles. Perhaps you're asked to fix a bug rendering them, or perhaps you're asked to insert logging code during map loading. Tackling these tasks would directly require gaining knowledge of some of the concepts mentioned above. Paradoxically, the best way to understand this code snippet might not involve looking at the code at all!

I did kinda just decree that a certain set of questions was most important when reading this code. But if you want an actual study, well, off the top of my head, there's "Questions Programmers Ask During Software Evolution Tasks." 



From their two studies, the authors identified 44 question types programmers asked themselves when doing some nontrivial tsk on a codebase. For example, question #3 is "Where is there any code involved in the implementation of this behavior?"  and question #27 is "Understanding a subgraph," is "How does this data structure look at runtime?" I went through and counted each question type that I could generously see being asked during reading-by-contributing; I counted only 13. 

Doing reading-by-contribution does have the benefit of turning a new codebase from a foreign land into familiar territory. But code familiarity is not code understanding. Make It Stick, the book from where I learned about retrieval practice, gives a nice anecdote of how learning does not happen automatically from exposure: a UCLA psychology professor fails a survey about fire safety, and then decides to find the fire extinguisher nearest his office. It was inches from the doorknob he turned every day for decades. Make It Stick calls this the "mere exposure" fallacy, and cites it as the reason why students who diligently reread a textbook are only getting the illusion of learning.

I'm not going to say that "just give a task somewhere in the codebase" is the optimal strategy for onboarding a newcomer. After all, there is still an approached as optimized for immediate benefit as for learning, and the practice is not the performance. I recognize that my personality type is more suited than most to this approach, as I feel a need to understand a lot about what my code will do before I make even a small change. But it is a tried and true strategy, and now you have some more idea of why it's true.


Footnotes:

1: Dark confession: I actually did put many hours into refactoring this 500-line function. It broke the game in a subtle way, we couldn't debug it, and so we pragmatically decided to revert.

Advanced Software Design Web Course: Now Open


Like this newsletter? My most polished content is in my  Advanced Software Design Web Course. The next run starts on 3/31 and is now accepting applications. 13/20 slots have already been claimed.

Testimonials from the most recent run:



 
Copyright © 2021 James Koppel Coaching, All rights reserved.