Code reviews will not save you from bad programmers

Code reviews will not save you from bad programmers

Code reviews are not a magic bullet which will slay all of your quality issues. Just like any other practice, they can be subverted for nefarious deeds.

Everyone thinks about the good and honorable people. They write good code and they demand good code from others when performing reviews. This post is not about them. This is about the dark side and people who will do whatever it takes to finish their project.

Imagine a small team with two people on it. They are recent hires and haven't really become familiar with the inner workings of the corporate code base. They have no idea what sort of techniques and idioms go along with it. They set off to work on a project.

Person #1 writes some code. It looks nothing like what the company would write in that language. It's code, and it runs, but it would be completely foreign to another typical engineer at the company. Person #2, sitting right next to #1, gets the review and approves it. Person #1 checks it in and proceeds to build upon it.

This process repeats. Sometimes #2 is the author and #1 is the reviewer. This goes around and around. Eventually you have a bunch of code which has all been reviewed but has no business being in the tree. It violates multiple fundamental rules about what to do and what not to do when writing code at the company, and yet it got reviewed! What gives?

The problem is that they managed to create a closed loop with no oversight. There were no checks or balances in their system. Nobody outside of that office got to see the code until it was much too late.

Years later, when they had both moved on and other programmers had moved in, you can imagine the consternation. None of it made any sense. There were blatant violations of policy: operator overloading, use of C++ streams for things other than logging, multiple inheritance, sleeping or connecting to production in unit tests, and so on. What a disaster.

Here's another gem pulled off by these same individuals. As the story goes, one of them wrote some Python code which ran in a huge block. Outside the block, it caught an exception and ignored it. That exception was SyntaxError. They said it was easier than trying to figure out whatever was going on.

They sent it out for review to someone who still had a soul and some sense of decency when it came to code standards. They quite rightly turned it down. They weren't going to stand for that kind of garbage.

At this point, the original author went back and put in the work to fix it, right? No more "catch syntax errors and ignore them", right?

Well, no.

The author found another reviewer. This reviewer didn't care about such things. The code was reviewed, approved, and checked in.

Code reviews may slow down the badness, but it won't change the fundamental situation: your code is only as good as the people writing it. If they are broken, your code will rot to resemble them.