Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

OK so a lot of code review is fairly routine. But occasionally there are changes that broadly impact existing complex, fragile code. In this situation, the amount of time it would take to verify the safety of the changes, absence of regression, etc. is excessive. Perhaps even exceeding the time it took to do the development itself.

What to do in this situation? Merge and hope nothing slips through? Do the best one can and try only to spot any obvious flaws (perhaps this is the most code review should aim for anyway?) Merge and test extra-thoroughly as a better alternative than code review at all?

share|improve this question
10  
What about running your test suite to make sure you didn't break anything? – Vincent Savard 4 hours ago
10  
what if there is no pre-existing test suite? -- How about writing one? – Robert Harvey 3 hours ago
1  
The test suite would definitively help. But the peer review and tests are complementary. I think it's not a good idea to replace one by the other. – Christophe 3 hours ago
1  
@MasonWheeler Eh, I'm not fooling myself, I'm in OP's situation at work. Doesn't mean it's not what we should strive for. – Vincent Savard 3 hours ago
3  
@MasonWheeler: Probably a conversation for another time, and you're referring to TDD specifically in that article, using assumptions that I don't think any self-respecting TDD'er would ever make, but I've done it both ways, and I consider the benefits of unit testing to be self-evident. – Robert Harvey 3 hours ago

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"

share|improve this answer
2  
Nice proverb. I'll need to remember that one. – RubberDuck 1 hour ago

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
share|improve this answer

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.

share|improve this answer
  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).

share|improve this answer

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.