Goal of this building block
Why review a (sub)product?
The DOT model describes many different activities and almost all activities result in a temporary or definitive partial product. This may involve documentation, but also, for example, a diagram, code, analysis or storyboard. These are the so-called stepping stones. They are the transfer of information from one research area to another. It is therefor communication.
Every person makes mistakes, even the most experienced technician has blind spots. In fact, experienced professionals are aware of how likely it is that they are missing something.
That's why we review each other's work. It is sometimes called the 'four eyes principle'. Two persons see more than one. It is supporting your team mates, "have look at it".
But there is also a less informal aspect to it. Documents are written to communicate something. And the target group must understand the text and be able to work with it.
A few examples:
- The client must agree with the purpose, budget, scope and preconditions of a project described in the project plan.
- The team members must understand how the planning is set up and what is expected of them.
- In larger projects, all software developers must understand the requirements because they have implement them.
- In the same larger project, the testers must define acceptance tests and therefore the requirement specification must also be clear enough for them.
- Anyone who has had to work with someone else's code can confirm it: clear code with clear comments prevents many mistakes. But the author of the code can no longer clearly see what is obvious and what needs commentary.
A final reason to review each other's work is that colleagues in a project get to know each other's work. Very useful because all the components team members make must be integrated into one product. Gaining knowledge and improving quality in the same activity.
The reviewer's focus
We always apply filters
You will probably recognize the following situation: you look at your watch to see if you still have time for a cup of coffee. You conclude that that is the case. Then someone in the queue of the coffee corner asks what time it is. You have to look at your watch again.
This happens because you looked at your watch the first time with the question: "Do I have time for coffee?", Which requires a calculation with the current time and the deadline you have. The current time is not 'stored in the memory', you forget it as soon as you know the answer to your question. You may have saved something like "I still have five minutes left".
Another example: You are looking for a girl in a crowd (café? Railway station? Classroom?). Do you see her? No. You continue. When someone asks: Who did you see? You do not know that. The only thing you know is that she's not there.
Checklists offer the right filter
When we search for something, we filter our observations to our search query. Translated into reviews, this means that it is extremely difficult to check for correct spelling and good design choices at the same time.
A checklist defines the focus of the reviewer and at the same time gives tips on how the reviewer can apply this focus. The chance that another reviewer with the same checklist will also find the same things has become bigger. In terms of research: the use of checklists makes the review more reliable.
By making checklists in advance, you specify not what has been made, but especially what should have been made. In terms of research: reviewing on the basis of checklists makes the product more valid.
source: http://geek-and-poke.com
Where it all began
This building block discusses an important apsect of the software inspection process. The process has been identified a long time ago and was very useful when there were no automated test methods.
The first formal method for inspections was developed in 1972 by Michael Fagan to produce zero-error code. This method is called Fagan Inspection. Later this method was extended to documentation reviews by Tom Gilb. There are currently a lot of tools available for so-called static code inspections like Soot, Findbugs, Infer and PHP Mess Detector.
Although formal reviews are less important with short development cycles and agile development, it is still an important skill when there are consequences (and costs) when errors are made in the development process. And it is an effective method to learn to reason about what is a good product.
Learning objectives
After this building block you can:
- specify which roles are useful for the review of a certain product
- specify a checklist of each role
- use the checklist in a review
De checklist voor review
General format for a checklist
There are three or four questions on a checklist for review.
The questions are closed-ended questions and have these two possible answers:
- Yes, (everything is fine)
- No (there may be a problem and the reviewer must describe what that problem is)
The following questions could work for each product:
- Does the product comply to the relevant standard?
- Is the product in accordance with the input?
- Is the product usable for the person who has to work with it?
- Is the product of good quality in itself?
In fact, these are four different focus areas for a review. It is best to give each reviewer one of these questions. So these are four different checklists with one question each. If the project is large enough, it is useful to have the author of the input document answer question 2 and have question 3 to be used by a reviewer who actually has to work with the product.
A more detailed checklist
The first three generic questions of the general format can be made more specific to help the reviewer focus and be thourough:
- Does the product meet the relevant standard?
- Name the relevant standard. E.g. a coding standard, uml standard, a template.
- Is the product in accordance with the input?
- Is the product complete?
- Is the product correct?
- Is the product not over-complete?
- Is the product usable for the person who has to continue with it?
- Ask yourself: do I understand this?
- Ask yourself: do I have enough information to continue working with it?
The fourth question can hardly be specified any further. It is about the added value of the product in itself. If a test specification is made based on the requirements and the above three questions have already been answered with 'yes'. Even then, in the specifications, bad choices can be made or techniques can be applied that are not usefull in that situation. This question is therefore less easy to answer by means of a systematic approach of criteria.
Specifieke checklist
The best checklist is one that has been made specifically for the type of product. Such checklists could be made for the project. For example, a checklist for requirements documents, for designs, for code, for testing.
Here we give an example for a functional design.
What must the reviewer do when a problem has been found?
Make a note of the alleged problem
Make a written report of what you have found.
The list of all the comments from all reviewers together, becomes an action list for the author. That has to be recorded, nobody remembers it all.
As soon as the answer to a checklist question is "no", or "probably not" or "I doubt it", there is something to report. The reviewer describes the problem.
If a cloud application such as Google Docs or Office365 is used, you can simply add review comments in the appropriate position. When it comes to offline documents, it is better to make a form in advance with a table and line numbers.
In this way the author gets something that is easy to merge into one list.
Restrict yourself, no solutions
It's preferred that the reviewer identifies a problem and is trying to come up with a solution. The author learns most by solving it him/herself and the author has most knowledge about the subject at hand and can best predict whether a solution will cause new problems.
So, the instructions for the reviewer are:
- In principle, do not describe the solution. E.g. "Rule 67. The color indication 'in bright colors' is not specific enough" is a good comment. The comment "Rule 67. The colors should be red and yellow" shows a solution and a bad one, because it is not really more specific than the original.
- For spelling errors you can give the solutions. E.g. "Reeding must be reading".
Note:
There are many ways to save time in writing down your comments. With a very detailed checklist, a problem description could consist only of a reference to the checklist, eg "Rule 76, checklist Code, point 4." Spelling mistakes can also be shortened by marking "SP reeding" or only a highlighting.
Review the product, not the author
source: alvarezporter.com/2013/05/throw-out-the-praise-sandwich/
In feedback about personal performance we have learned to ‘sandwich’ the comment. First a positive thing, then the improvement issues and then again a positive ending.
In inspections this will take too much time. But of course there is a danger of authors loosing self-confidence if there are a lot of comments and of reviewers getting over-confident and arrogant because they make a lot of comments.
The basic attitude of the reviewers must be that they perform a temporary role; that next time they will be in the author’s chair and would like to have all the help they can get to find those blind spots that everybody has. Be objective, clear and open-minded.
Image: Johnny Hawkins
Individual assignment
The attachment contains an inspection checklist for requirements. The criteria (checklist questions) are derived from the acronym SMART that is often used for objectives and requirements.
The assignment is to review 5 requirements using all questions in the checklist and to note all comments in writing.
Preparation:
- If you are working in a project it is most effective to divide the requirements among the team so that in the end all requirements are thoroughly reviewed.
- Also discuss with each other what the inspection record will be.
Note: Usually in a project, each reviewer will review all requirements with a specific focus. In this first exercise we want you to experience how detailed the review of a single requirement can be. Providing of course, that one takes the meaning of SMART seriously.
Group assignment
Several students in the project group have reviewed something in the previous assignment.
The group assignment is to merge the comments to one big list and discuss them in a meeting:
- If more reviewers have reviewed the same requirements with the same checklist: compare notes and merge your comments to one list.
- Assign a chairperson and send all inspection comments to that person.
- The chair merges the comments into one review record.
- Discuss the results in a meeting
In this exercise you have two items to discuss with each review comment:
- the quality of the comment:
- the problem and not the solution,
- the work and not the person,
- specific enough to help the author.
- does the author of the requirements understand the issue?
- Discuss after the meeting what you would do different next time that you do a review.
Sources
Gilb, T., & Graham, D. (1993). Software Inspection. Harlow: Addison-Wesley.