What do you do when code review is just too hard?

The premise of the question is, frankly, astounding. We suppose that there is a large change to fragile, complex code, and that there is simply not enough time to review it properly. This is the very last code you should be spending less time on reviewing! This question indicates that you have structural problems not only in your code itself, but in your methodology of managing change.

So how to deal with this situation? Start by not getting into it in the first place:

• Identify sources of complexity, and apply careful, heavily reviewed, correct refactorings to increase the level of abstraction. The code should be understandable by a fresh-out-of-college new employee who knows something about your business domain.

• Identify sources of fragility; this could be by review of the code itself, by examining the history of bug fixes to the code, and so on. Determine which subsystems are fragile and make them more robust. Add debugging logic. Add assertions. Create a slow but obviously correct implementation of the same algorithm and in your debug build, run both and verify that they agree. In your debug build, cause rare situations to occur more frequently. (For example, make a memory allocator that always moves a block on reallocation, or always allocates a block at the end of a page, or whatever.) Make the code robust in the face of changes to its context. Now you don't have fragile code anymore; now you have code that finds the bugs, rather than causes the bugs.

• Write a suite of automated tests. Obviously.

• Don't make broad changes. Make a series of small, targeted changes, each of which can be seen to be correct.

But fundamentally, your scenario is "we have dug ourselves into a hole of technical debt and each complex, unreviewed change digs us deeper; what should we do?". What do you do when you find yourself in that hole? Stop digging. If you have so much debt that you are unable to do basic tasks like reviewing each other's code then you need to stop making more debt and spend time paying it off.

One of the primary goal of a code review is to increase quality and deliver robust code. Robust, because 4 eyes usually spot more problems than 2. And the reviewer who has not written the additional code is more likely to challenge (potentially wrong) assumptions.

Avoiding peer reviews would in your case only contribute to increase fragility of your code. Of course, reinforcing testing with a solid and repeatable test suite could certainly improve the quality. But it should be complementary to peer review, not a replacement.

I think that complexity must be understood and mastered, and the full peer review is the occasion to share knowledge and achieve this. The investment you make in having more people understanding the strength and weakness of the fragile code, will help to make it better over the time.

A quote to conclude:

"If you want to go fast, go alone. If you want to go far, go together"

Solve the larger problems that are causing code review to be too hard.

The ones that I've spotted so far:

1. No unit test suite
2. Complex code merges that could be avoided by more sensible code structure and delegation of coding duties
3. An apparent lack of rudimentary architecture

1. You can send the code review back and tell the developer to break it up into smaller, more incremental changesets, and submit a smaller code review.

2. You can still check for code smells, patterns and anti-patterns, code formatting standards, SOLID principles, etc. without necessarily going through every detail of the code.

3. You can still perform tactical code inspections for proper input validation, locking/thread management, possible unhandled exceptions, etc. at a detailed level, without necessarily understanding the overall intention of the whole changeset.

4. You can provide an assessment of overall risk areas that you know may be impacted by the code, and ask the developer to confirm that these risk areas have been unit tested (or ask that he write automated unit tests, and submit those for review as well).

If you think that the code review is too hard, because it changed brittle code that is near impossible to change without breaking it, then you have a problem. But the problem is not with the code review. The problem is also not with unit tests, because brittle code cannot be unit tested! If your code was unit testable then it would have been split up into small, independent units, that each can be tested, and that work together well, and that's exactly what you don't have!

So you have a heap of rubbish code (aka "technical debt"). The worst thing you can do is starting to fix that heap of rubbish code and not finishing the job because that way you'll get an even bigger heap of rubbish code. So the first thing is to get your management to buy into fixing it and to finish the job. Or you don't. In that case you just don't touch it.

When you fix it, you extract one unit from the code, make it into something that has well-defined and well-documented behaviour, write unit tests for that unit, code review it, and pray that nothing breaks. And then you do the same with the next unit, and so on.

The tricky bit comes when you run into bugs. Your rats nest of code will do the wrong things in some cases because things are so brittle and complicated, things will go wrong. As you extract units, the remaining code will become clearer. (I had a case where after some refactoring, a function started with "if (condition1 && condition2 && condition3) crash ();" which was exactly the behaviour before refactoring, only clearer. I then deleted that line :-) You will see weird and undesired behaviour clearly, so you can fix it. On the other hand, that's where you must change the behaviour of the existing code, so that needs to be done carefully).

In this situation, the amount of time it would take to verify the safety of the changes, absence of regression, etc. is excessive.

Code reviews shouldn't be primarily aimed at correctness. They are here to improve code readability, maintainability and adherence to team standards.

Finding correctness bugs during a code review is a nice byproduct of doing them, but a developer should make sure their code works perfectly (including non regression) before submitting it for review.

Correctness should be built in from the start. If one developer isn't able to achieve it, have them pair program or figure out a plan with the whole team but don't treat it as something you can add as an afterthought.

Unfortunately, there's not really much you can do about this at the point of code review other than get another cup of coffee. The actual solution for this issue is to address the technical debt you've accumulated: fragile design, lack of tests. Hopefully, you at least have some sort of functional QA. If you don't have that then there's always praying over some chicken bones.

From my experience, I would strongly recommend you to cover your code with a fair amount of tests, both unit and integration, BEFORE any changes are made to the system in question. It's important to remember that nowadays there's a very good number of tools for that purpose, doesn't matter the language you're developing with.

Also, there's THE tool of all tools for you to create your integration tests. Yes, I'm talking of containers and specially of Docker and Docker Compose. It beautifully provides us with a way of quickly setting up a complex application environment, with infrastructure (database, mongodb, queue servers etc) and applications.

The tools are available, use them! :)

If you're not content to ship with buggy/non-functioning software and fix it later, then V&V effort SHOULD be longer than development effort!

If existing code is fragile, then a first question is "should you even be changing it?" Management need to make a call on whether the cost/risk of redesigning and reimplementing this code is greater than the cost/risk of fixing up the wobbly pile of junk. If it's a one-off, it may be easier to just patch it up. If there are likely to be more changes needed in future, taking the hit now to avoid more pain in future may be a better decision. You need to be raising this with your management, because giving your managers good information is part of your job. They need to be making that decision, because it's a strategic decision which is above your responsibility level.

More answers are addressing how you got to this point. Many of them give some suggestions to remedy the situation, but I'd like to throw my answer in to give the short answer.

What to do when code reviews are "too hard?"

1. Go back to the main line code branch
2. Write tests for the functionality you've refactored (e.g. functional tests)
3. Get the tests to pass
4. Merge the tests into the code that's "hard to test"
5. Do the tests still pass?

Yes

You developers were great! Cats back for everyone!

No

Keep refactoring and adding test coverage until the tests are passing.

Welcome to the world of legacy software development.

You have 100s of thousands, millions, 10s of millions of lines of code.

These lines of code are valuable, in that they produce a revenue stream and replacing them is infeasiable.

Your business model is based off of leveraging that code base. So your team is small, the code base is large. Adding features to is required to get people to buy a new version of your code, or to keep existing customers happy.

In a perfect world, your huge code base is unit tested up the wazoo. You don't live in a perfect world.

In a less perfect world, you have the budget to fix your technical debt -- break your code down into unit testable pieces, do exensive integration testing, and iterate.

This, however, is paying down debt without producing new features. Which doesn't match the business case of "reap profits from existing code, while modifying it in order to generate incentive to upgrade".

You could take huge chunks of code and rewrite it using more modern techniques. But everywhere you interact with the existing code you'll be exposing possible break points. That hack in the system that you got rid of actually compensated for a quirk in a subsystem you didn't rewrite. Always.

What you can do is act carefully. You can find some part of the code that you actually understand, and whose behavior and interaction with the rest of the system is well understood. You can modernize that, adding unit tests and making its behavior even clearer.

Then find the parts of the rest of the app that mainly interact with it, and attack them one at a time.

As you do so, you can improve the subsystem, adding features that the customers are willing to pay for.

In short, this is the art of the possible -- making changes without breaking things that provide a business case.

But this isn't your question. Your question is, "I am doing something that is huge, and likely to break stuff, and how do I follow best practices?"

When doing something huge, it is true that if you want to do it reliably you end up spending more effort tracking down bugs and fixing them than you do writing it. This is the general rule of software development: writing stuff is easy, making it work flawlessly is hard.

You probably have a business case hanging over your head, where you have promised to some stakeholder that this massive change goes in. And it is "done", so you get pushback on saying "no, this isn't done, it just looks like it".

If you have the power and the budget, actually spend the effort generating confidence that the change works, or simply reject the change. This is going to be a matter of degree, not kind.

If you don't have that much power, but still have some, try to insist that the new system is unit testable. If you rewrite some subsystem, insist that the new subsystem is composed of small parts with well specified behavior and unit tests around them.

Then there is the worst case. You go deeper into debt. You borrow against the future of the program by having more code that is fragile and more bugs in order to get the feature out now, and damn the consequences. You do sweep-based QA to find the worst issues, and ignore the rest. This is actually sometimes the right answer from the perspective of the business, as it is cheapest now. Going into debt to generate profits is a valid business strategy, especially if clearing the debt via bankruptcy (abandoning the code) is on the table.

A large problem is that rarely are the incentives of the company owners aligned with the decision makers and the programmers. There tends to be lots of pressure to 'deliver', and doing so by generating nearly invisible (to your superiors) technical debt is a great short and sometimes medium-term strategy. Even if your superiors/stakeholders would be best served by not creating all that debt.