Art by angelchama
If you're anything like me, you probably like writing software quickly. But you also like the code being good. It's not exactly rocket science that one often comes at the expense of the other, at least in the short term. In the long term, of course the better code allows you to keep a steady pace, and ultimately saves you time. In the short term though? Who cares about the code review? LGTM! We can always just fix it later.
Then "later" turns out to be two years from now, when you don't even remember writing this code, much less how it works. In the meantime, you've usually got a footgun (or tripmine) that everyone's had to learn to work around. This aggravates developers, competent and incompetent, and can really kill the momentum on a project if there get to be too many. I'm a firm believer that if you want to build great software, the best strategy is to build an environment where working on the software is fun. That's how you'll get the most creativity, ambition, and productivity out of your dev team. Your project should be "Yes! I get to add a feature to <X>!", not "Fuck. Please don't let this feature need changes to <X>." Some areas of software are just inherently shittier than others, but I think you can go a long way with good tools and good code reviews.
This article isn't about how to create that type of environment. At least, not directly. We'll be discussing a couple strategies you can use, as a developer, to make your programming experience more enjoyable, and your code better, without dedicating additional hours to things like code reviews, style guides, or extended conscious thought. In other words, how to write better code without doing anything.
Note: Many of these strategies extend the timeline for actually delivering features, but they do not impact real productivity. Another way to put it is that these strategies improve code quality at the cost of latency, but have negligible impact on throughput. Perhaps that's a spoiler. Let's get into it.
I'm a big believer that timing is everything, and that includes the timing for code reviews. It's generally quite easy to switch tasks when you first show up to work. You're fresh, you're (sometimes) rested, and you haven't yet had a chance to fill your brain with the context needed to solve a problem. It follows then, that you should open PRs at the end of the day, right before you go home, because it creates a natural resting point before you go and review your own PR. You are reviewing your own PRs, right? Assuming you do "self-PRs", then you should specifically be opening a draft PR at the end of the day.
Then, in the morning, you should review it, make changes as needed, then publish it. Then forget about it until tomorrow and start working on something else. Resist the temptation to see if the principal engineer is tearing you a new one in the comments until at least the next day. This means all PRs are going to take at least two days to merge. That's fine, because you're not actually spending any additional time on them. You're just using better time. In practice, this might even save time as you're not burning cycles on context switches, waiting for comments, or watching automated tests fail.
If your coworkers know you're doing this, it's also a huge benefit for them, because they know exactly how much time they have to get their feedback in. It gives them enough time to be thorough and avoid randomizing themselves, without giving them so much time that they forget about it entirely. What if it's a large PR that takes multiple days to review properly? First off, don't make a large PR. Second, if you must, then reviewers can leave comments indicating they're working on it and block the PR from advancing until they've had a chance to finish.
Uh, skill issue? You can just branch off the PR, optimistically assuming it will get merged as-is, and fix any conflicts later. Ideally, under this system, you won't build up much of a backlog because PRs should generally be getting merged within a couple days. The time spent resolving merge conflicts between your own changes should be pretty minimal. The time you're otherwise wasting twiddling your thumbs while waiting on a code review is very significant.
Sometimes, you will build up a large backlog of pending PRs. Maybe your coworkers are slacking (politely: "busy"), or you've pushed multiple large refactors in a short span of time. Let me know if you find any strategy out there that actually handles this situation effectively. I've yet to see one. Typically this represents an organizational problem with your team or company and there's not much you can do about it as an individual developer.
My Advice: Open draft PRs at the end of the day, and review them first thing in the morning. Don't look at your own PR again until the next day. If you need work from a pending PR, just pull it in to your branch and resolve the conflicts later.
Sometimes, you realize a problem is hard. Sometimes really, really hard. Like cache invalidation! Sometimes you sit there, just thinking, staring at your screen, hands resting on the keyboard, prepared to type... something. This can really interrupt your flow and make it hard to keep going. A lot of engineers are prone to getting nerd sniped by these problems, dropping everything else to think about them. In some cases, their massive brains pull through and get them where they want to go. Most of the time, however, all this thought and energy ends up wasted. Either because it turns out the problem is a known, hard CS problem, or because so much time was taken in thinking about it that the requirements have changed. Even when it "works out", it's often a poor use of time.
There's another, more minor form of this though, which is when you're writing a system and partway through, you realize that it's ugly. You don't know exactly what's wrong with it, or how to do it better. But you know there's something wrong with it. And you can easily waste hours trying to figure out a "better" solution. Many engineers in these situations get frustrated and start throwing more abstractions at the problem until it's "fixed" (read: hidden). Other times they'll sit on the change for weeks or even months, waiting for that perfect solution to arrive. Then the other, good changes they've got cooking end up getting burned to the stovetop because they've been left there for so long.
I ran into a situation like this recently at work. I had an enum class that was included in a header file, but it seemed like the wrong place for it. My brain got stuck on it. It really bothered me that I didn't know where else to put it. But before I wasted hours on it, I recognized that I was stuck, and I walked away. Instead, I opened the PR with the ugliness there, and waited to see if anyone commented on it. Maybe it didn't need to be solved at all. Or maybe the answer would come to me when I revisited the problem with fresh eyes. Or perhaps someone else had an obvious solution that I was missing.
For the next several days, I left the PR open, responded to feedback, and periodically thought about this problem. But I never spent more than 5-10 minutes each day thinking about it. By day four, I realized that this problem did not have an obvious and elegant solution, and because it's so minor, I should just accept some ugliness. I moved the enum out of the header file, and simply hardcoded the values in the other places that needed it, wrote a comment, and moved on with my life. On paper, this seems ridiculous. Four days! For "where to put an enum with 5 members"! But really, it had zero impact on productivity, because the actual time spent was negligible. At most I spent 30 minutes in a week on this issue. This is what I meant by "latency" vs "throughput". The only true cost was "four days of getting work done without these changes".
There's no shame in walking away from a problem. As a developer, your job is not to solve problems. This is a common misconception. Your job is to deliver and improve software. That often requires problem-solving skills, but nobody's going to pay you to sit in your office all day and think about beautiful code. It always needs to translate into results at some point. Users aren't going to care about your fancy round-robin scheduling algorithm. What users will notice is "Why can't I pair a fourth Bluetooth controller to my computer? This shit sucks!"
In matters of software, management is usually wrong. You get paid the big bucks to change reality until they're right. That requires high-quality thought, and high-quality thought requires frequent breaks.
My Advice: If you're struggling with a problem, walk away and try again tomorrow. Seek opinions and feedback on the problem in the meantime. If you haven't arrived at a solution within a few days, consider giving up and moving on.
The only code with no bugs is no code. Therefore, any time you delete code, you are reducing the number of bugs in the software. Get in the habit of deleting code, pruning files, and cleaning up interfaces as you go. When you see more removals than additions in the diff, that should feel incredibly satisfying to you. You will never have time dedicated to refactoring and rewriting, so do it as you go instead. Code quality is measured not in terms of how many lines you have, how many tests you've written, or how many libraries you use. Code quality increases the more functionality you provide, and decreases the more concepts you have to understand. The best code is the code that doesn't exist because you didn't have to write any. The only way to achieve a concept count of zero is to cut the feature. So remember your ABCs: Always Be Cutting.
What exactly do I mean by "understand more concepts"? Well, let's look at a "simple" example.
// If I catch you writing code like this, I will personally come to your house
// and force you to implement multiple inheritance in x86 assembly.
const iceCreamFlavorBuilder = new BuilderFactory();
iceCreamFlavorBuilder.on("fieldAdded", (fieldName, _options) => {
// we want to enforce that iceCreams are always tasty
iceCreamFlavorBuilder.decorateField(fieldName, { tasty: true });
});
iceCreamFlavorBuilder.addField<string>("flavorType", {
validate: (v) => ["vanilla", "chocolate"].includes(v),
defaultValue: "vanilla"
});
const vanilla = iceCreamFlavorBuilder.build({ flavorType: "vanilla"});
const iceCream = { flavor: vanilla };
Concept count: 10 BuilderFactory, iceCreamFlavorBuilder, field addition event, field decoration, field addition in general, validate, field default values, the validation function itself, ice cream flavor, ice cream,
As you can see here, even though there are lots of abstractions at play, none of them provide any actual value. What if I want to add strawberry flavored ice cream, and include the number of calories? Well, shit. Now this whole pattern breaks because the author of the fieldAdded event didn't think that far ahead. This "extensible" system is actually harder to extend than something basic. And do you even understand what the above code is doing? What happens if validate returns false? What happens if somebody adds properties to the builder after we've already made objects with it? Is the defaultValue required to pass through the validation function? Compare that to a much simpler solution:
class IceCream {
public flavor: string;
public tasty: boolean;
public static readonly legalFlavors = ["vanilla", "chocolate"]
public constructor(flavor = "vanilla") {
if(!legalFlavors.includes(flavor)) {
throw new Error(`Illegal flavor: ${flavor}`);
}
this.flavor = flavor;
this.tasty = true;
}
}
const iceCream = new IceCream("vanilla");
Concept count: 2. You have to understand IceCream and probably IceCream.legalFlavors. Doing this the easy way isn't just less work, it's literally better.
My Advice: Try things the stupid and straightforward way first. Add abstractions only after the low-IQ approach has become painful, and only add enough to alleviate the pain.
As you can see, there are a lot of ways to make your life easier and increase your code quality without doing more work (in fact, sometimes doing less work). Hopefully you apply these strategies in your own working life. I've been using these tactics for the last few months with lots of success so far. I hope you find them useful too.