7

We were using git gerrit for manual code review . but recently we are planning to integrate sonarqube in our Jenkins integration server. Do we still need manual code review? Or we can stop manual code review and sonarqube is enough. I would appreciate if you share your experience with sonarqube.

2
  • 4
    What are the features of SonarQube regarding semantics of names?
    – mouviciel
    Nov 28, 2014 at 9:41
  • It covers 7 axes of code quality 1.Architecture & Design 2. Comments 3. Coding rules 4.Duplication 5.Unit Testing 6. Potential bugs 7.complexity. www.sonarqube.com
    – Sharif
    Nov 28, 2014 at 16:18

2 Answers 2

20

Yes, you should keep code reviews.

SonarQube, like any other automated tool, is great as a tool which helps pointing out some of the possible problems, including problems which are not easily seen by humans.

Duplicate code, for example, can be checked automatically, as shown by Visual Studio or SonarQube, while a reviewer may not easily see that the same piece of code was copy-pasted and slightly modified afterwards.

In the same way, style checking can and should be automated, because an automated checker will perform better than an human. Static analysis is another great example.

But then, there are issues which can hardly be automated. For example, would an automated tool find something wrong in this JavaScript code?

// The price should be incremented when a rebate is available.
if (this.rebates.findFor(this.customer, this.product)) {
    this.customPrice -= this.computeRebate(this.customer);
}

On the other hand, any reviewer will immediately find two issues:

  • The comment is plainly wrong and misleading. We are not incrementing the price.
  • Even if it were correct, the comment remains useless. If the two lines of code are not clear in the context, a minor refactoring can make it explicit enough to be able to get rid of the comment.

Another benefit of code reviews is the ability to teach junior programmers. For example, an automated checker will find this piece of C# code written by a beginner programmer correct:

var result = new List<Day>();
foreach (var day in days)
{
    if (day.HasAppointment)
    {
        result.Add(day);
    }
}

return result;

while any more experienced developer will suggest rewriting it like this:

return days.Where(day => day.HasAppointment);

Even if an automated tool will do such suggestions (after all, it is possible to programmatically detect procedural code which can be rewritten using functional style), would this tool be able to explain to the beginner programmer the caveats of such rewrite (lazy evaluation)?

3
  • 3
    I wish I knew more beginner programmers who understand the imperative logic (like in your "junior code" example) behind lambda queries… Nov 28, 2014 at 9:47
  • 4
    Another benefit of code reviews is the ability to teach junior programmers. because seniors surely can not learn from other seniors or even juniors... Nov 28, 2014 at 10:23
  • Resharper detects code that can be rewritten as LINQ quite often. Nov 28, 2014 at 11:29
6

IMO, Manual code reviews helps in forming a shared understanding of the codebase, reducing dependency on a single or a small group of individuals. It may also teach a thing or two in programming style/language features for the reviewer reviewing the code. It works great if everyone reviews every other person's code or pair with someone senior who is good at reviewing.

So unless it is a single person reviewing the code of 10+ individuals and you introduce sonarqube to automate some of that process, I would rather keep manual code reviews for the sake of the points stated above.

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.