The Tip of the Iceberg

In my last patch, I knew that the return type (for a status code) of my newly created function wasn’t right. I didn’t want to trigger a cascade of issues, though, so I left it. Better to maintain consistency with the variable as it was declared in the original code, I thought. Besides, I didn’t know how far I’d need to propagate the change, and I didn’t want to get into another mega refactor that I couldn’t handle.

My mentor, though, asked me to fix that issue and resend the patch. That had me collecting an unsigned value (a u16) in a signed int. Sparse wasn’t complaining, and I knew it was being assigned one of two #defined values so it would be promoted with no problem, but it was still kludgy. The right thing to do was convert the declarations and return types throughout the file. Did I trigger a cascade of issues by doing that? Yes, I did.

When I changed the declaration within the helper function, I triggered warnings from the style checking script (called checkpatch) because, throughout that function, lines were indented by three spaces instead of by eight space wide tabs. There had been a problem with that line all along, but now that I had touched it, it was in my patch. I could have changed the indentation for that line and let it go, but it was better to fix the indentation and line lengths throughout the helper function. I did that, and surprise–more checkpatch warnings. Using tabs instead of spaces triggered the “too many leading tabs” warning. Besides, some of those lines were indented so deeply that there was no room left for a variable name between the indentation and the 80 character limit. The seemingly simple change from ‘u16’ to ‘int’ had led me to another function extraction.

Now that I had seen the bottom, though, it wasn’t nearly as scary as I had imagined it to be. There was a clearly defined logical chunk I could take, the innermost of three nested loops. I was able to make two of the variables local to the new function, which meant I only needed to pass three parameters. And after I moved the extracted lines to the left and squashed some, I was left with about fifty. I had a nice, neat refactor that I could be proud of.

This is how change is made, and how opportunities are found. That little red flag, that one slightly irritating thing that’s out of place, tells us where to dig in, tear down, build, create, and win. Don’t ignore it, tolerate it, or shrink from it. Put something awesome in its place.


Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s