Example Review Issues List


Work Product Inspected:  Hall of Fame class skeleton
Author/Team: Pete / Sunnyside Software
Reviewed by:  James /  Sterling Software

Date

 Oct 22

Start Time

 6:45

End Time

  8:10

Severe Problems / Issues

Accessor method "getHall()" breaks encapsulation by returning a reference to underlying data representation.
HashMap is not an appropriate implementation because it doesn't maintain an order among its elements, which is necessary for hall of fame.

Minor Problem / Issues

Seems wrong to have separate setter methods for name and score as you would never want to change a name in the hall of fame without changing the score.
There is no toString() method or other way to obtain a printable representation of the hall of fame.

Trivial Problems / Issues

addplayer() should be addPlayer().
"ranking" is misspelled in addPlayer() method header.


Recommended Action

[ ] Accept

[ ] Accept with minor rework

[X] Reject

[ ] Unable to complete Review


Review Issues List - Examples

Example of poor issues list - items are too vague.


Review of Class Diagram by Team SkaterSoft
Reviewer: Sam Student
Date: 11/5/2003

Did Well
  1. Problem Decomposition
  2. Reusable
Needs Improvement
  1. Classes seem to have missing methods
  2. Class names are not informative
  3. Some dependencies appear wrong or misplaced.

Example of good issues list - items are clear, concrete, and specific.


Review of Class Diagram by Team SkaterSoft
Reviewer: Joe Student
Date: 11/5/2003

Did Well
  1. The design is decomposed well - Nearly all the methods perform just a single, focused function that could be implemented in less than thirty lines.
  2. The solution has been designed for reuse - The Hall of Fame class in particular is loosely coupled, independent, and could be reused in a different game with no changes.
  3. etc.
Needs Improvement
  1. There is no method to calculate score.
  2. There is no method to update tiles left.
  3. Level attribute should be private.
  4. Method name "restart" is unclear: is that restart a game or just restart a level?
  5. Tile class should have an equals() method.
  6. Board class appears to aggregate Tile, but is shown as dependency.
  7. array is a poor choice of data structure for ChallengerList, a queue would better match the problem domain.
  8. etc.