Peer Review Form for Iterable IntRelation ========================================= Reviewer information -------------------- Name: Group number: Date: Review Targets -------------- This review focuses on the iterator-related code in * IntRelationArraysIterable.java * IntRelationListOfSetsIterable.java * IntRelationIterableTestCases.java Criteria -------- Rate each of the following five items on an integer scale from 0 through 2, and motivate the score (also score 2). You can also provide further feedback on that item (what you found nice, what can be improved). Enter these results in the template, below the cut line. At the end, there is also room for additional feedback. When filled out, submit this entire file as Review Report and submit the total score (in the range 0 - 10) as Grade for the peer review. ---- 1. Good modular structure Indicate whether the structure of the implementation has good modularity. Good modular structure has many benefits: improved confidence, improved verifiability, improved adaptability, etc. Here, we focus on two aspects: * Whether the classes IntRelationXxxIterable are defined by extending IntRelationXxx (rather than copying all that functionality), and implementing Iterable. That is, the class definition looks like this: public class IntRelationXxxIterable extends IntRelationXxx implements Iterable { ... only code for the iterator ... } * Whether the the class that implements Iterator is defined as a private inner class, but NOT as a local or anonymous class. That is the code looks like this: @Override public Iterator iterator() { return new YyyIterator(); } private class YyyIterator implements Iterator { ... implementation of YyyIterator ... } NOTE: Good modular structure is also important for test cases, like this: public abstract class IntRelationIterableTestCases extends IntRelationTestCases But we do NOT grade that here. (End of NOTE) 2. Both structures explained above were used in both the Arrays and ListOfSets classes. 1. On at least one occasion, the explained structure was indeed used, but also on at least one occasion it was NOT used. 0. The explaiend structures were not used at all. Motivate the score; clearly point out relevant defects. ---- 2. Convincing implementation of Arrays iterator Indicate whether the implementation of (the iterator in) IntRelationArraysIterable is convincing. Imagine that this software goes into a nuclear power plant, aircraft, or medical equipment (with you living nearby, on board, or as patient). Correcness of the iterator also requires that it works as expected, regardless of how often hasNext() is called (including zero or multiple times) before each next() call. 2. You are convinced, by reading the code, that it is correct, AND the instance variables of the iterator were explained in a comment, AND in case of multiple instance variables, their intended relationship is explained in a comment (preferably, but not necessarily, in a rep(resentation) invariant). Performance (time, memory) is NOT an issue here. 1. Exactly one requirement for score 2 fails. 0. More more than one requirement for score 2 fails. Motivate the score; clearly point out relevant defects. ---- 3. Convincing implementation of ListOfSets iterator Indicate whether the implementation of (the iterator in) IntRelationListOfSetsIterable is convincing. Imagine that this software goes into a nuclear power plant, aircraft, or medical equipment (with you living nearby, on board, or as patient). Correcness of the iterator also requires that it works as expected, regardless of how often hasNext() is called (including zero or multiple times) before each next() call. 2. You are convinced, by reading the code, that it is correct and efficient, AND the instance variables of the iterator were explained in a comment, AND in case of multiple instance variables, their intended relationship is explained in a comment (preferably, but not necessarily, in a rep(resentation) invariant). Performance IS an issue here! Just traversing ALL pairs (a, b) and returning the ones for which areRelated(a, b) holds is NOT efficient. The List-of-Sets representation is useful not only to reduce memory cost, but also to improve iteration performance. 1. Exactly one requirement for score 2 fails. 0. More more than one requirement for score 2 fails. Motivate the score; clearly point out relevant defects. ---- 4. No unnecessary data or code duplication Indicate to what extent unnecessary data and code duplication have been avoided in the iterator classes for Arrays and ListOfSets. Here, unnecessary data duplication would be, for instance, to create a local copy of the list or set of pairs to be traversed, and then traverse that copy. Unnecessary code duplication would be, for instance, code for finding the next pair to be delivered both in the constructor and in next(). Or, the code that checks for the availability of a next element, occurring both in hasNext() and in next() as condition for throwing NoSuchElementException. Such dupclication is avoided by using auxiliary methods and by using hasNext() inside next(). 2. No unnecessary data and code duplication. 1. Either unnecessary data duplication or code duplication, but not both. 0. Both unnecceary data and code duplication. Motivate the score; clearly point out relevant defects. ---- 5. Sufficient test coverage Indicate whether the test cases in IntRelationIterableTestCases provide enough basic coverage. N.B. This includes testing for robustness (expected exceptions). 2. The iterator and each of its methods hasNext(), next(), and remove() are well tested, both for 'good weather' (precondition satisfied) and for 'bad weather' (precondition violated). Good weather test cases include these boundary cases: empty iteration, singleton iteration, iteration that involves a full relation with extent > 1. If the only missing test case concerns an unimplemented remove(), then that is OK (because it is easy to check that code by reading). 1. The iterator is tested for important situations, but one important situation was missed (e.g. 'bad weather'). 0. Multiple important situations were missed. Motivate the score; clearly point out relevant defects. -----8<----- cut line -----8<----- 1. Good modular structure Score: Motivation: Feedback: 2. Convincing implementation of Arrays iterator Score: Motivation: Feedback: 3. Convincing implementation of ListOfSets iterator Score: Motivation: Feedback: 4. No unnecessary data or code duplication Score: Motivation: Feedback: 5. Sufficient test coverage Score: Motivation: Feedback: =========== == Total score =========== == Other remarks (e.g. about readability, design, etc.): (End of Peer Review Form)