• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

code review vs static analysis

 
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Roy's post got me thinking about the difference between code review and static analysis.

The static analysis vendors say static analysis is a type of code review. I use static analysis for two purposes:
1) To get me looking at potentially troublesome code. Bad practices tend to cluster. As a human, this code is worth more time.
2) To ensure certain practices never make it into the codebase. Why should a human need to look for a DateFormat object as an instance variable.

I certainly don't think static analysis replaces code reviews of course. I'm curious what others think on this. Both the people from SmartBear and more broadly.
 
Smart Bear Support
Posts: 8
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
At Smart Bear, we use both FindBugs and PMD for static analysis and we still review every line of code. There are certain things that static analysis tools are great at and some things that they just cannot do. They can detect most bad idioms (like your DateFormat example). They can detect inconsistent code (you checked for null before dereferencing a parameter once, but not every time within a method), which probably indicates a bug. They can detect complex code (by computing cyclomatic complexity) and flag it for further review or refactoring.

They cannot reason about whether the code works, conforms to the spec, and is maintainable. In my mind, this is the primary function of code review. Here are some questions that go on my mental code review checklist:

1) Do the unit tests validate the specified behavior?
2) Are there missing test cases?
3) Did the unit tests change? If so, was this an intentional change or a "get the unit tests to pass" change?
4) Can I understand the code?
5) Are variables well-named?
6) Are the comments sufficient?
7) Is there a better way? Especially a better way that makes use of some common code. Static analysis can detect duplicate code, but that's not exactly what I'm looking for here. I'm considering alternative approaches that would simplify the code by using some common library code.

For more thoughts:
Can static code analysis replace peer code review?
 
Ranch Hand
Posts: 2187
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The phrase "code review" is typically used to describe a human process, not a "computing" process. In reality, it most likely has different meanings for different organizations. In my experience, "code review" is a meeting of one or more engineers, one or more programmers, and a project manager or business analyst. The purpose of the meeting is to review the "design" of a software application or service.

Static analysis of code is a "computing" process and can be used to create documentation to be reviewed in a code review meeting. Or, it can be one of the steps that occurs before or after a code review, and maybe depend upon how well the code is written.

Basically, you can use static analysis and code review together.
 
Smart Bear Support
Posts: 6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Just one additional thought - you can use static analysis and code review together: http://blog.smartbear.com/the_smartbear_blog/2009/05/how-peer-code-review-fixes-the-falsepositive-problem-with-static-analysis-tools.html

HTH,
Gregg
 
Jeanne Boyarsky
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Brandon,
Great thoughts. I like how you use code revie for "readable maintainable" code and not just "code that works at the moment."
 
Greenhorn
Posts: 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Static analysis checks are only as good as the rules that the analysis uses to find stuff. Code Review encompasses many other types of "checks" that are not or even could not be codified into an automated rule. They may include reviewing code for functional integrity - something that static analysis does not typically focus on. Static analysis usually finds flaws in coding practices, such as logic problems that suggest the code is incorrect or possible exceptions in the code and doesn't understand features.

Static analysis does do a subset of code review and can do it on a much larger scale (even through most every path in the code). It's a good idea to use both static analysis and manual code review together.
 
Don't get me started about those stupid light bulbs.
reply
    Bookmark Topic Watch Topic
  • New Topic