Code Reviews: Best Practices
|
|
|
|
Friday 25 February |
|
|
15:20 - 16:00 |
Maintenance Block |
Lecture 6 |
Code Reviews: Best Practices |
Gerhard Brandt |
|
This lecture addresses the following
questions
|
|
— |
How to write code that's readable
and understandable ? |
|
|
— |
Which tools can you use to make this
easier ? |
|
|
— |
How to understand already existing
code ? |
|
|
|
| |
|
Starting points for this lecture:
|
| |
|
Other people have engineered code for
you.
|
| |
|
|
— |
It's your honor to adjust this code
where it shows |
|
| |
|
|
|
suboptimal behaviour ( = fix bugs )
|
| |
|
|
— |
You learn from their ingeniosity and
apply your experience |
|
| |
|
|
|
as you and others contribute new code
|
| |
|
Approaching a foreign body of code
top-down:
|
| |
|
Read it in increasing level of detail
|
| |
|
|
— |
Read File/Directory Structure |
|
| |
|
|
|
(like Design Patterns, Interfaces,
Libraries, makefiles)
|
| |
|
stay on top - dive only as required!
|
| |
|
( = don't try to read 100k lines of code
from the beginning to the end )
|
|
* High-level Orientation in an unknown
body of code
|
| |
|
|
— |
Documentation and its Generation |
|
|
* Use the command line, like: Simple
heuristics
|
| |
|
|
* cvs: Watch what happens during
checkout
|
| |
|
|
* ls: directory structure
|
| |
|
— |
Condense code to structural
elements: {} , ; |
|
| |
|
— |
ref: Cunningham W., OOPSLA 2001
Software Archeology Workshop |
|
| |
|
|
— |
Real-time access to CVS |
|
| |
|
|
— |
View Changes, Diffs, Tags, ...
immediately |
|
|
* LXR - Linux Cross Reference
|
| |
|
|
— |
Perl script that generates xref'ed
source code in HTML from C++ |
|
| |
|
|
— |
Not real-time on CVS - rerun by
webserver about once a day |
|
|
* Generating documentation from code
|
| |
|
|
— |
many different tools - incompatible
formats |
|
| |
|
|
— |
Used with ROOT based applications (eg.
H1OO?) |
|
| |
|
|
— |
Classes to be documented must be
included in ROOT |
|
| |
|
|
|
(ClassDef ?,
ClassImp ?
Macros) |
| |
|
|
— |
Need code that sees ALL classes to
generate complete documentation |
|
| |
|
|
|
(eg. executable that links everything)
|
| |
|
|
— |
Non C++ Files not documented |
|
| |
|
|
— |
Bugs (eg. inline functions don't
work correctly) |
|
| |
|
|
— |
Unofficial outlook: THtml2 |
|
| |
|
|
|
|
— |
ROOT team choice: rewrite doc tool
from scratch, incl. C++ parser etc. |
|
| |
|
|
|
|
— |
more features: more output formats,
code browsing from CINT cmdline, ... |
|
| |
|
|
— |
good results for un-enriched code |
|
| |
|
|
— |
too many bells and whistles? |
|
| |
|
|
— |
Graph generation tool from BellLabs? |
|
| |
|
– graphical representation of code
structure
|
| |
|
|
— |
used by Doxygen for its graphs |
|
| |
|
|
|
|
|
— |
Notation for type and scope? |
|
| |
|
|
— |
example: messages/error logging |
|
| |
|
|
— |
often old/suboptimal solution |
|
| |
|
|
* Headers, Initialization
|
| |
|
|
— |
Checking Contributions by others |
|
|
— |
Available in other languages: C++
cppunit |
|
|
— |
normally used for test driven
development |
|
| |
|
-> not identical to correctness checking
|
|
— |
Example H1OO?
- H1 Fast Validation |
|
|
— |
Check code based on changes in
physics variables |
|
|
— |
Compare set of observables from
identical data |
|
| |
|
but reconstructed from two different
releases
|
|
— |
Differences must make sense from
physics POV |
|
| |
|
-> if not, infer indirectly to problems
in the code
|
|
— |
Very simple implementation, great
success for our purposes |
|
|
* Enforcing Coding standards: Code
Beautifiers
|
| |
|
|
* Jalopy (a java code beautifier)
|
| |
|
|
* Code analyzers PMD (Java)
|
| |
|
|
Testing coverage reports (Clover,
JBlanket)
|
| |
|
— |
Spinellis D., Code Reading, Addison
Wesley 2003 |
|
| |
|
— |
McConnell?
s., Code Complete, Microsoft Press,
2nd Ed 2004 |
|
| |
|
— |
... Test Driven Development |
|
|
|
|
|
