8

At our workplace, we use Agile Scrum method. However, we don't really follow it religiously.

But, we do very thorough code reviews. The problem with code reviews is that it's takes a while to document the problems with the code, and send it back to the developer. I suppose they want the code reviews to be documented because it might be good for auditing purposes, and also proves that the code reviewers is actually doing his/her job properly.

But it is seems take a lot of time out of the day where we analyze someone's code, and document mistakes. It would have been better to spend the time doing new development/research/support/maintenance.

How can we speed up code reviews without sacrificing thoroughness of the code reviews?

3
  • 5
    Surely you're better off spending the time on the code review now instead dealing of the accumulated ball of poor quality code in the future. And if documenting all the errors is taking so long - you need to address why are there so many errors
    – HorusKol
    Aug 5, 2016 at 14:00
  • 2
    A fine detail: are you looking to not sacrifice thoroughness, or is it possible that you have slightly more thoroughness than you actually need, and are willing to trade it for speed. The latter is a balancing act, so you typically have more options available.
    – Cort Ammon
    Aug 5, 2016 at 15:48
  • Maybe this will help: yegor256.com/2015/02/09/serious-code-reviewer.html
    – yegor256
    Aug 15, 2016 at 2:20

5 Answers 5

8

A couple of suggestions:

Code Quality Tools

It is worth thinking about using automated code quality tools like Findbugs, PMD and Checkstyle.

Ideally get the team to agree on a set of coding standards and implement them as templates in the various code quality tools. Then run the tools from continuous integration and possibly even fail builds when the quality standards are not met.

This won't replace code reviews, but it will hopefully reduce discussions at code reviews relating to coding style, formatting, etc.

Just the fact that you have clear standards will help reduce the time spent on code reviews.

Code Review Tools

There are some good code review tools around.

An example is Crucible.

These kinds of tools are great for helping with the facilitation of code reviews and they also help a lot with code reviews run remotely (say when a team member is working from home).

3

Are the goals that the review can be audited? or to prove the auditor is doing his/her job? We document them as new tasks/discoveries on our Scrum-board and some just picks them up.

I think the main goal of code reviews is knowledge sharing and finding recurring code mistakes, not the documentation, unless you need it by law :)

Checklist based code reviews: This e-book has a great practical and light-weight strategy for peer code reviews. For a summary if this strategy read my answer to another question on SE.

Pair programming: Some teams think pair programming is a good alternative to code reviews as the code is reviewed by another person during the act.

5
  • Pair programming is good, but not adequate to replace a review - the review is done coldly, while the pair are both in the heat of the moment
    – HorusKol
    Aug 5, 2016 at 14:01
  • According to research at Cisco (based on 2500+ code reviews) 61% of the code reviews do not reveal any defects. So i think in the majority (61%) of the cases pair programming would be sufficient. Depending on the actual real life risks extra review might be necessary, but in relative low risk environments it might be just good enough. Results may very and opinions will differ on this topic. You could try only pair programming and do a root-cause analysis on issues. This to find out if a more formal review would have found this issue and update your process accordingly. Aug 5, 2016 at 14:18
  • 2
    Hmm.. That means reviews find defects 39% of the time - if 2 out of 5 reviews find a defect, that's definitely a reason to keep doing them. Also, exactly what is a defect in terms of that study? An actual bug/regression, or some design decision that incurs technical debt?
    – HorusKol
    Aug 5, 2016 at 14:31
  • If you put it like that maybe you are right :) still I would suspect the defect count is lower after pair programming. I don't know by head what defect means in terms of that study, but the study is referenced and summarized in the linked e-book of my answer if you are really interested. (although hidden behind a signup wall I thought the read was worth my time) Aug 5, 2016 at 14:43
  • At one time i would leave the last bug I found in the code for the review, as a test to see if the code review team would find it. They only found it in 50% of the cases, so the absence of bugs found in a code review is a red flag to me.
    – pojo-guy
    Sep 19, 2016 at 16:02
1

A few years back my team was in a major time/resources crunch. We solved this question by assigning a senior (me) to do the code reviews.

I used Findbugs to narrow my search and identify obvious bugs, but didn't trust it fully because it was prone to report false positives. For example, it would frequently claim that the logger was and unused variable when I could see that the code logged heavily.

Once I had used findbugs to this extent, I reviewed code in question manually in context to rule out false positives and to find issues the findbugs is ill-equipped to find (Spelling and grammatical errors).

I would spend 30 to 60 minutes prepping for code review, then 10 minutes in the actual meeting. Total resources expended averaged under 1 programmer hour. We never failed to find and eliminate at least three bugs in any meeting.

1
  • I certainly agree that preparing for a code review is key! Sep 12, 2016 at 3:06
1

You could consider using elements of Test Driven Development.

You might find the code reviews quicker, and no less thorough by having the tests created prior to the development, so that the developer can continuously analyse his or her progress until it meets the requirements of the pre-approved test.

This could certainly reduce the back-and-forth nature of the review.

0

Stop doing code reviews and start pair programming. If the purpose of code reviews is to improve the quality of the code and while helping team members increase their domain knowledge, then pair programming is a much more pro-active form of code review. It also builds quality earlier into your development process.

Speed up = improve efficiency. So you're asking how do we increase efficiency without sacrificing effectiveness. One big down-side of code-reviews is that the team members doing the code review first need to understand the business problem as well as code they are trying to review. This is often done by re- actively reviewing the user story and code after everything is already "done."

A hands-on way to get the same knowledge is through pair programming. You're killing two birds with one stone; implementing the user story and doing a thorough CR in parallel. There's your efficiency gain (and its usually more effective too since most adults learn better through hands-on activities vs passively studying on their own).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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