(c.f. Hypercritical)
Code reviews are hard — both from the perspective of reviewing code, and having your own work reviewed.
You need to distance yourself from the code, and see how it integrates with the rest of the system. You have to be critical, and that doesn’t come naturally to a lot of people: you have to look at what the code is, and see what’s wrong with it. That second part is where I think a lot of the difficulties lie, at least for me.
I don’t find it too difficult to be critical, per se: I have always viewed my own work with suspicion, and I see the flaws and shortcuts that I have had to take in getting to a solution for a particular problem. Ever since I was a kid, I’ve fostered a healthy sense of skepticism about everything, including my own work.
There’s a dark side though.
I don’t think I’ve ever had really strong opinions. When faced with the choice between two equally good options, I tend to weigh the two in my mind; worse, I tend towards looking at things from all perspectives; inasmuch as I can make a decision, I will delay it until I’ve seen it from all sides.
This has led to the perception that I am indecisive, or that I dither in my opinions. I’ve always thought that the truth sometimes lies somewhere in between two opposing viewpoints, and have tried to discern where that midpoint may be— even to the point of looking at my own work and seeing where I may have been wrong to making any particular technical decision somewhere.
The problem lies with the intersection of this desire for balance and consideration for all arguments with the large unknowns of software. Sure, I can make an easy decision when the technical good of a particular decision is evident, and there are no real opposing viewpoints — that isn’t the case most of the time when it comes to building software.
That makes it difficult for me sometimes to look at someone else’s code critically, as a critical assessment of anything involves making some sort of value judgement on it. Saying something is somehow wrong implies there is a right thing, or what you believe to be the right thing.
It’s also temptingly easy to just focus on the surface details: code style, commit messages, etcetera. Those things are helpful in trying to understand the changes being reviewed, true, but the true benefit of reviewing code really is in looking at it deeply and critically.
Did the author really need to hard-code this value here? I mean, it is expedient, but at what cost? How about this bit of duplicated code? Would it make sense to generalize this bit of code, or should we keep it as is?
I know that all of those knotty questions are best asked of the author of the code in question, so that we can elicit a better understanding of the thinking behind the code— at least those bits of context not apparent from commit messages and comments in the code. But I still think it goes back inevitably to a value judgement: is the change good for the overall health of the codebase?
And that question I find hard to answer.
It’s All A Consensus Protocol
The real answer, I increasingly find, is that the value judgements we make when we build software should be derived from consensus: a consensus of opinions from the team, or even the industry at large. How we derive that consensus is particularly bothersome to me sometimes.
I would love for us in the industry to get to agreement on certain practices empirically, and not because of some think piece somewhere by someone. Has it been shown that x works, and in what contexts? What are the trade-offs (and there will be trade-offs)? Instead, a lot of things we do in industry is received folk wisdom clothed in respectability from being called “best practices”.
I don’t know— maybe all of this is my own lack of rigor from not having a formal background or education in the field, something I regret sometimes. Like I said: I do have a suspicion of my own work, and therefore my own knowledge, and I am acutely aware of how much I don’t know. Therefore, who am I to make a judgement about a piece of code I’m reviewing?
Again: it’s always consensus that I fall back on. What do my peers say about factory methods? Or empty exception handlers? Or even whether duplicated code is truly a bad thing? Is there consensus? If there is, then I best head in that direction: a thousand other people couldn’t possibly be completely wrong, I guess.
All I know Is I Know Nothing
It’s incredibly difficult to look at code that someone else has written and making those value judgements, especially if you feel like you don’t know jack. It’s scary to look at other people’s code and criticize it, because you feel like your perspective is incomplete, or that you can see why it makes sense to do it the way they did. I struggle every time: I make up my mind on how something should have been done, only to later come to the opposite conclusion, because I could see their possible reasons for the code as it is.
I have my biases, for instance: I lean towards server-side processing code and building backend APIs, because that’s what I’ve done for the most part in my career. So, if I take a look at software where a lot of the business logic is in the client side, I instinctively react negatively: why not do it on your backend? But, after some contemplation, I can see why there’s a lot of client-side logic. Maybe it’s because they wanted to do offline processing, or maybe they thought the system needed to be fairly standalone, and that would have been the most expedient way to do it.
But are my own instincts valid? I don’t know.
And that’s the rub: I find it personally difficult to make these value judgements sometimes, at least definitively. I tend to make those judgements with caveats and exceptions and conditionals.
As I’ve gotten more experience over the years, I find it becomes even more difficult to be definitive, as I find myself looking at the pros and cons more particularly.
I do inevitably make a value judgement of the code I review, though, so at the end of it all I do make a decision whether the code passes muster for me, but every decision I make I am always a little unsure of. “I may be wrong” has slowly crept into my vocabulary, and I don’t think of that as a bad thing completely, but it always makes me unsatisfied of my own technical decisions, knowing that at the end of it is, maybe, a load of bullshit.
Previously: Passports and Visas