Cause it's the single biggest thing you can do to improve your code.
One of the most effective, and humbling, ways of identifying errors in your application is to ask for a peer review. It’s a great supplement to the testing you normally do[1] on your project. Peer reviews can catch 60% of the errors in a project. The author of Code Complete points out that peer reviews actually have advantages over traditional software testing.
“They are more cost effective on a per-defect-found basis because they detect both the symptom of the defect and the underlying cause of the defect at the same time. Testing detects only the symptom of the defect; the developer still has to isolate the cause by debugging.”Peer reviews come in lots of forms: inspections, walkthroughs, pair programming (“real-time” review), or even sending an email to ask a design question to a trusted colleague.
The general idea is simple, with two key points:
- Read the code
- Involve someone else
1. Read the code
How often do you go back and review your code once you have it compiled and running? “It works, I’m done.” That may be necessary/tempting on tight project timelines, but it hurts long-term maintainability.Every time I sit down and actually read through my own code, I find something that can be improved: comments that don’t match the code, unused variables, redundant logic, inefficient data structures, and all sorts of “UPDATE” comments highlighting things I know I could do better but didn’t have the time to be perfect.
Reading through the code is a fast and easy way of identifying these items -- and hopefully encourage you to fix them while you're looking at them.
2. Involve another person
A fresh perspective on your code is almost always valuable, especially if there’s code you know is “tricky” or “clever”[2]. What may make perfect sense to you at the time you are coding may look like gibberish when Phase 2 rolls around in 6 months. Your peers can help point out those potentially confusing spots. (Think of the times that you've inherited someone else's code and it took you a whole day just to figure out what was intended by the original author. Now, how would you feel if that original author was you?)There’s also a human nature aspect – since you know your peers are going to be reviewing your work, and especially if they’re smart peers, then you’re likely to spend some time cleaning up your code just so you don’t embarrass yourself!
How to conduct an effective peer review
Peer reviews can be more or less formal, from two people looking at a section of code together via shared desktop, to a formal “code inspection” that involves several people with distinct roles.The objectives of an inspection, in priority order, are:
- to identify errors in the code/algorithm
- to review for maintainability[3]
- to provide a shared learning experience for the author in when and how to apply techniques to improve code quality
The team needs to be aware it's not just a meeting where you get everyone together in a room and start reviewing. It involves preparation. In a code inspection, there are some distinct roles:
Reviewer: Prepare in advance by reading through the code thoroughly (can't skip this, it's the most important step). Focus on identifying any and all errors -- logical holes, if without else, style issues, "clever" code, bad comments. Come prepared with corrections, comments, and questions. Feedback should be directed at the work product, not the author, but it should be to the point.
Author: First step is to briefly explain the overall design to the group. If the design is complicated, then you may need a "pre-meeting" before the reviewers go off and read the code. Then during the inspection, walk the group through the code, line by line, explaining what is happening. Usually, the parts that are difficult to explain verbally are the parts that need the most work. As the author leads the group through this, the reviewers chime in. The author listens to the feedback, takes notes, and then makes his/her own decision later on what changes need to be implemented and which can be deferred.
Moderator: Can really help streamline the process. His/her objective is to keep it moving, usually by cutting off any defensive conversations ("well, you're wrong, I did it this way because…") or any reviewer that's getting too detailed (or too mean!). That's not the point. In formal settings, the moderator can also serve as a scribe that creates an action list for follow-up.
For example, one reviewer may have an issue with your variable naming scheme (you didn't preface your module-scope variables with m_ e.g. m_iNumWidgets), and you may decide that's less important and can be addressed later as time permits. But someone else may identify a function that needs serious refactoring, and you may decide that needs to happen immediately. Depending on the author's level of expertise, they should probably choose to accept at least some of the changes.
Make sure the section being reviewed is short enough to be manageable, and worthwhile. A time limit of 2 hours is about right. Unless the author is really junior, don't spend any time on simple things like read routines. Go right to the hard stuff -- this is where your reviewers are more likely to catch errors or holes.
Developers are people too
Finally, before you go into a code inspection, read Effective Code Reviews Without the Pain.Keep in mind the objective of peer reviews isn’t to beat down the author! It has nothing to do with establishing who is the alpha programmer in the group. It has nothing to do with showing how much smarter you are by criticizing someone else’s hard work.
The purpose is to create the best quality product for both our customers and our colleagues who may inherit the work someday. And in the end, the quality of our work is one big reason our customers keep coming back.
No comments:
Post a Comment