Transcript Peer Reviews

Software Reviews
Introduction/Motivation
• When creating written documents, it is a good idea to have someone
else proof read your work. Oftentimes an error that escapes you is
easily picked up by another reviewer looking at the work from a
different perspective.
• Software reviews are the practice of having someone else look over
your work for the purpose of finding errors and/or deviations from
standards and specifications.
• Software reviews are a form of collaborative development.
Types of Software Reviews
• The concept is simple, but the terminology varies widely:
Five Types of Software Reviews will be discussed here:
Summary and Distinguishing Characteristics
• Inspections – Inspections have well-defined roles: moderator, reader, recorder, author
and inspectors; and a well-define process: planning, overview meeting, preparation,
inspection meeting, rework, follow-up and optionally causal analysis.
• Technical Reviews – Compared to inspections, technical reviews are less formal and have
fewer roles. For example, during a technical review the author may also be the reader.
• Walkthroughs – Walkthroughs are organized and ran by the author of the work. One of
the goals of a walkthrough is to make sure everyone in attendance understands the
work.
• Deskchecks – Deskchecks don’t require a meeting. With a deskcheck, the author sends
the work to one or more reviewers, who read the work independently and send back a
list of errors found and general comments. The author is free to ignore the feedback
received. There is no requirements for consensus with a deskcheck.
• Pair Programming – Pair programming is where two programmers work together at one
workstation. One controls the keyboard and is in charge of writing the code. The other
reviews each line of code as it is entered. One thinks tactically; the other thinks
strategically.
What can be reviewed?
• Any work product can be reviewed. Good candidates are:
•
•
•
•
Project vision statement
Project charter
Software Requirements Specification (SRS)
Architecture and Design
• Source Code
• Test plan
• Test cases
• User documentation
Inspections complement dynamic testing
• Several errors can be identified at once. With testing errors usually
are discovered and fixed sequentially.
• With inspections you can find errors early, closer to their source. With
testing you have to wait for there to be working code.
• With inspections you can find types of errors that would be difficult
or impossible to find with dynamic testing. For example, code that
doesn’t meet standards set for maintainability can’t be identified with
dynamic testing.
Inspections save time and money
Another look at the cost savings
Not convinced inspections are a good
investment for your team?
• Track how time is spent on several projects, some doing inspections,
some not.
Daily Time Report
Lines of Code (added or removed) : ________________
Hours spent on inspections: ________________
Hours spent on rework (locating or removing defects): ________________
Hours spent on other development activities
(requirements, design, coding and test): ________________
Comments: ____________________________________________________
______________________________________________________________
Other Benefits of Software Reviews
• Improved product quality and maintainability
• Provides for well-defined and measurable exit criteria for a phase in the
project life-cycle.
• Cross-training and skill enhancement. Participants can learn other parts of
the system outside their immediate area of responsibility. Also, it provides
an opportunity for junior analysts to learn technical skills from more senior
analysts.
• Continuous process improvement through causal analysis. Performing
inspections creates the opportunity to improve the standard development
process. If inspection results show the same type of error reoccurring,
action can be taken to address the root cause. For example, if errors are
repeated found in code copied from stackoverflow, a new rule can be
instituted such that any code used from stackoverflow must be coded by
two programmers practicing pair programming with both programmers
clear on how the copied code works.
Should managers attends software reviews?
Guiding Principles for Software Reviews
• Don’t use results for performance evaluation. This is why managers
shouldn't participate in inspections.
• Critique the work not the worker. The atmosphere of an inspection
meeting should be positive with the focus on improving the work
product and not the errors of the author.
• Make software reviews part of the culture. True professionals are
open to constructive criticism. (Similar to the idea of Egoless
Programming)
Inspections
• Inspections are a proven technique for finding errors and improving
the quality of work products.
• Inspections are a formal method of review. They are considered
formal because there are well-defined roles and activities.
• Roles: moderator, reader, recorder, author and inspectors. All
participants act as inspectors but their may be inspectors that don't
have any other roles.
Inspections – Roles
• Author - person who creates the work product being inspected.
• Moderator - person responsible for organizing the inspection process
and keeping the meeting on track.
• Recorder - person responsible for recording defects and technical
issues during the inspection
• Inspectors - persons responsible for inspecting the work product and
finding defects
• Reader - person responsible for narrating during the inspection
process
Inspections – Getting Started
• Prior to an inspection, the author performs the work and creates a work product
according to the organization's procedures, rules, guidelines and standards.
• There should be written organizational standards that authors follow when creating the
work and inspectors follow when judging the quality of the work. Otherwise, what is and
isn’t an error becomes more subjective.
• When a work product is ready for review, the author passes is along to the moderator
who performs an cursory check to verify that the product is ready for inspection.
• If the product is ready for inspection, the moderator selects 2 or more qualified
reviewers and prepares the materials to be inspected. The moderator makes sure that all
inspectors have access to the work product, its source requirements, and applicable
procedures, guidelines, standards, etc.
• Inspectors should have checklists for common errors. Checklists may be specific to the
type of work product being reviewed. For code inspections checklists may be language
specific.
• An optional overview meeting may be schedule during which time the inspection team is
introduced to the work product being inspected. An overview meeting isn't necessary if
the inspection team is familiar with the inspection process and the product under review.
Inspections – Overview meeting
• During the kick-off meeting the moderator and author introduce the
materials to be inspected. Inspectors should already have been
trained in the inspection process.
• During the kick-off meeting the moderator may provide additional
special instructions for the particular work product being inspected.
• If the inspection hasn't already been scheduled, it is scheduled at the
conclusion of the kick-off meeting.
Inspections – Prep
• Materials are distributed several days in advance of the inspection
meeting.
• Inspectors inspect the work product in private and make a list of
potential defects.
Inspections – the meeting
• Before the inspection meeting the moderator should determine that
everyone is adequately prepared. If participants haven’t had a chance to
review the work before the inspection meeting, the meeting should be
rescheduled. Inspections are less effective if materials haven't been
reviewed in advance. Many errors are found during this review or
preparation time.
• During the meeting the reader paraphrases the work product. Participants
raise questions and note potential defects during the reading of the work
product.
• It's important not to try and solve errors during the meeting. There isn’t
enough time during a 2 hour meeting to both find defects and determine
their solution. The group may discuss whether or not a potential defect is
in fact a defect but the meeting is not the best place to discuss or debate
solutions to the defect, style issues or changes to the standard
development process. Someone may offer a solution but not more than a
few minutes should be spent trying to solve any one problem.
Inspections – the meeting
• The recorder documents defects as they are raised. For each defect
the recorder should document the location of the defect and its
category/type/severity. Example severity levels: critical, major, minor
and trivial.
• The recorder should also keep track of suggestions for improving the
standard software process.
• The moderator is responsible for keeping the meeting moving
forward in a productive manner.
• At the end of the inspection meeting the list of defects are given to
the author and the moderator.
Inspections – at the end of the meeting
• At the end of the meeting, process metrics are gathered. Potential
metrics include:
•
•
•
•
•
Total preparation time of all participants.
Meeting duration.
Total meeting effort (number of participants * meeting duration)
Lines of code (or other measure of product size) inspected.
Defects found and information about defects (severity, type, etc)
Inspections – after the meeting
• After the meeting the recorder gives the author the list of errors
found.
• The author performs any rework necessary to correct defects found.
• The moderator follows-up with the author to make sure that the
rework is performed and decides if another inspection is warranted.
• If substantial rework is required a follow-up inspection may be
scheduled. Otherwise, the product has passed the inspection and the
work is considered complete.
• Experience during the inspection may suggest ways to improve the
standard software development process including the inspection
process. Suggestions for improving the standard development process
are forwarded to the process owner.
Coding Standards
1. All methods should have inline comments that explain, at a minimum: the
purpose of the method, the expected input, the process and the outputs
including any exceptions that are part of the interface.
2. Variables and data types should have descriptive names. Variables should have
appropriate names based on their function or purpose.
3. The scope of variables should be as tight as possible.
4. Code should not be repeated unless it provides extra clarity that outweighs any
reductions in maintainability and efficiency. Code will be considered repeated if
there are more than 10 lines of nearly identical code in more than one place.
5. User input should not directly be used in constructing commands to the
operating system, SQL statements, or other commands/statements that require
authentication and authorization to submit.
6. Code that parses or assumes knowledge of the structure of XML or JSON data
should include an example document or object in the comments of the method
or class containing the code.
7. Any outside libraries used must be on the approved list of external library
dependencies.
Technical Reviews
Walkthroughs
Deskchecks
Pair Programming
References
• Design and Code inspections to reduce errors in program
development". IBM Systems Journal, 1976.
• Advances in Software Inspections, July 1986, IEEE Transactions on
Software Engineering.