3 ms·
Anyone who has ever come along in a code review and told me my code “isn’t up to standard” will inevitably start a very long game of attrition with me. Here’s t
by withinboredom 3y ago
Anyone who has ever come along in a code review and told me my code “isn’t up to standard” will inevitably start a very long game of attrition with me. Here’s the thing: there’s no such thing as “standard” software architecture (until someone writes a new kind of software architecture called “standard”).
I’ve seen and used everything from MVC to DDD to TDD to MVVC to whatever React is to reactive to event-oriented to actors to whatever you can imagine. There simply isn’t a “standard” way to do anything in this industry. If you hired me, you hired me for these different perspectives on problem solving, and that goes for all of us.
A code review simply isn’t the place to enforce your unique perspectives on someone else.
Sometimes, there is simply a better way to do things. These might even be new ideas in the code base. If it looks like it is, hopefully the author will bring it up with the team beforehand. If the reviewer changes their mind in the code review… well, that’s not the authors problem. The reviewer should rewrite it and the author review it.
Actually, that’s exactly what I suggest if someone ever tries that with me (usually when I’m the new guy). Please submit your own PR showing me how it’s done. I’ll move on to another ticket.
Usually, about half way through their implementation, they’ll see why I did it the way I did it and approve my PR. It only takes one time before they start asking “why” instead of assuming they know everything; every line of code exists for a reason, after all.
In one case, one specific dude (bless his heart) kept blocking PRs after we discussed everything before hand. I had to get HR involved, and other engineers. Dude just didn’t like being wrong and would throw tantrums when he was. He eventually got fired after people realized they could do what I was doing to stop the nits he said was a blocker. Nobody cared if a variable should be renamed from “SameFactory” to “EqualFactory”.
Ah, but code quality! It’s so subjective. High code quality, to me, is easy to maintain code. Is the intention clear, do the comments reflect reality, is it easy to read, but more importantly, easy to change? Will changing a line in the module break 15 other sibling modules? God I hope not. Dependencies should be obvious. And, no, I’m not writing an interface if there is exactly one implementation. That’s ridiculous.
For some people, they want layers. More layers of abstraction than a layered cake on a wedding day. To them, that’s good code quality! Some people think high code quality is beautiful code. The kind you frame and put over your fire place to admire with a glass of wine.
There’s no objectively “high quality” code in existence.
- JoeAltmaier 3y agoI'd like to agree. But there is one part of that distinction that is real: there is such a thing as low quality code.
- withinboredom 3y agoI think there is a such thing as “bad code” but not low quality code. Bad code can be detected by automated tooling and be improved through simple refactoring.
- JoeAltmaier 3y agoI'd say it this way: bad code doesn't robustly handle all the use cases, mis-interprets inputs sometimes, or is a mess to read and debug. Yes, it matters what the code looks like. Knew a guy, named everything in his code a letter of the alphabet. a,b,c and when he got to z started za, zb etc. That was 'bad code'. Or, it was 'Low Quality Code'. Nobody wants junk like that. Even if it passes tests, is robust etc.
- withinboredom 3y ago> Nobody wants junk like that. I can think of a few tools that do that to your PHP or JS code, on purpose. They even cost quite a bit of money (obfuscation). Sounds like you might have gotten a good deal there ... /s
- ravenstine 3y agoYeah, gotta love it when someone uses terms like "standard" and "quality" but definitions of those are written nowhere. Quality is almost never objective either; some cultures like "overripe" bananas and some consider the best bananas to have no spots, and those picking bananas need a different basis for what makes a good quality banana to the buyer. Unless programmers diligently document their standards (which they rarely do adequately), they shouldn't be surprised if the other programmers is dumbfounded by being told their code isn't up to standard.
- 3y ago