Author. Gamer. Thinker.

Friday, September 14, 2012

The Nature of “Good” Code

Developers who are young and still starting their career often focus on semantics and formatting to help separate good code from bad code. They lack the ability to easily parse and absorb code at a glance and rely on formatting differences to create their definition of what is considered good or bad code in their eyes. I’m here to tell you that where you put your curly braces or your line length doesn’t offer clues as to the quality of the code.

When introduced to a new team with an existing codebase, you can expect most developers to start off making tactical changes. They don’t know the codebase well enough to have absorbed it all in one day, but if they have a reasonable amount of talent and knowledge, they’ll pick up on patterns that they’ve seen before and quickly find a reasonable place to insert logic that achieves the goals of a specific task set out for them. Meeting the requirements of a task doesn’t require understanding the entirety of a codebase. We want to cheer on these people for adapting and being productive quickly. Hold the applause for a moment.

Tactical changes—changes that don’t account for the overall health and quality of the codebase—are at best benign and at worst harmful. It is the job of the technical lead and other developers of that codebase to review new changes and offer strategical advice to that developer as they grow in their understanding of the codebase. I expect developers on my team to transition to more strategical thinking at their own rate—depending on their experience, the size and complexity of the codebase, and how many other people are in the codebase changing it from day to day. Developers who cannot make the transition as a strategic partner or who don’t have the same vision as the technical lead of the project can be a drain on the project as a whole.

Tactical code has a place in every project. In the interest of time and budget, we are often forced to create code that is strictly tactical in nature. We call this type of code “technical debt” because we know that someday we’ll have to pay to bring it into the fold.

We each may have our own strategic goals and I believe we should each set them for ourselves. The enduring goals of every codebase I contribute to include being maintainable, concise, specific, efficient, and self-documented. Strategic code furthers these ends. Tactical code completes a task without regard for any other factors. I’ll elaborate on my goals in the hopes that you may find your own.

Code that is concise needs to be legible. I’m not trying to incite people into using ternary operators and embedded incrementors in their array indexes. I’m trying to say that methods, functions, and classes should be short, to the point and directed at achieving a small amount of functionality. This makes the code more testable and legible. If we name things appropriately, we should see what a method is setting out to do and then see it do it by calling other methods named in such a way that we know what is in the box before we look. Long methods and classes take time for developers to mentally parse and absorb their meaning. With descriptive names, the code should read easily and without the burden of comments that will only go out of date and eventually be inaccurate. 

This leads me to self-documentation. When you are writing a framework for others to use, you may have to document APIs to offer a starting point, but even something as public as a framework should be usable by the names of classes and methods. If others can’t tell the behavior of a method or purpose of a class by its name then it must be renamed. If it cannot be renamed to be meaningful, then it should be broken up into smaller pieces or discarded entirely. 

Code should be specific in that it doesn’t set out to achieve every goal and make waffles, too. Achieve the goals set out for your application and don’t write a line of code more. Too often, I see people adding methods, interfaces, and classes for some future day that doesn’t come. IDEs are powerful and extracting an interface when you need it is easy and modern mocking frameworks don’t require you to have an interface to create a test. Make them when you genuinely need them. All the fluff only clutters up a codebase and makes it more difficult to understand and trace through. We aren’t in the 90s any more when refactoring code was a more manual process. I’m begging you: stop creating stuff we’ll never use. A smaller codebase is better for everybody.

Efficiency is something that is easy to go overboard on. I’m saying that, strategically, code should be efficient, but I’m not saying that every line should be optimized, rather I’m saying that it shouldn’t be wasteful. Assigning default values to be reassigned in a constructor? Creating order n^2 complexity functions? Bubble sort? Seriously. There’s no reason to be wasteful of memory or CPU. I’m not advocating that you profile every method and try to tune it to perfection. I’m simply asking people to use common sense. If the application needs to be profiled later, there will be less noise. Efficient code can be legible and should be. However, attempting to reduce one line of code while joining strings with commas is a pet peeve of mine. It won’t make it more legible. We’re computer scientists, we can read a loop with a single if statement in it.

When I say that code should be maintainable, I mean that it should be both testable and tested. Code that is testable can be refactored and, once tested, can offer new developers insight into the code. I am not an advocate of Test Driven Development. I believe TDD lends itself to tactical development. Some of the least strategic code I have ever seen has been done TDD and has also been the most error prone. It is easy to live in the moment of the one test and the one goal and lose track of the big picture. You can do TDD and be strategic, but the less experienced developers are easily focussed on edge cases and a specific goal. With mocking frameworks being what they are, you can force nearly any code to be tested but simply having a test doesn’t make code good either. Even if you can’t agree with me about TDD, I think most developers can agree that code must be written to be testable. Whether you write your tests first or second, I will leave that up to you.

You may find and value other strategic goals in your codebases. I think defining those for yourself and sharing them with your team may help everybody on your team be more successful.

1 comment:

  1. I've often started tactical in the past because I've been new to projects with an established case base where bug handling has gone off the tracks. Either because senior developers are more worried about new features or the business has quit asking for second tier features they suspect they won't get in the next decade. You can get to strategic quickly and create good will by being willing to hunker down, do some analysis, and make a reccomendation about which tactical bits you can fix while you're new that might give he project a bit more shine, even if it's spelling errors, the -st -nd endings for numbers that don't align., or flashing out missed unit tests. You buy a bit of time to get familiar with the code and strategic possibilities and the codes history, which can be helpful. Good article, and I fully agree less is usually best. Over engineering is distracting and slows teams down.