Inspector?s Name:Brad Graham

Date Started:22 Feb 2001

Date Completed:23 Feb 2001

Project Name:Student Data Base

Time Report:

Time spent on Familiarization with the Code:10mins

Time spent on Filling in Findings Report:60mins

Time spent on Actual Inspection:60mins

Findings:
 

#	Description	Line #	Class Name	Associated Checklist Item #
1	Setting this to null is also done in the constructor, it only needs to be done in one of those places	12, 23	ListImpl	Simplicity
2	The constructor initializes first but not last. With reference to issue #1, initialization is only needed in one place.	16,23	ListImpl	Consistency
3	Line indented too far	33	ListImpl	Consistency:2
4	This comment refers to a non-existent argument	29	ListImpl	Maintainability: Misleading comment
5	Textbook pattern? I don?t know if this matters, but I usually see pre or post increment operator in these cases: ++count;	50	ListImpl	Simplicty:3
6	Indenting, it looks like tabs were expanded differently for this machine. Possibly a combination of tabs, spaces, unix, and windows formats getting mixed.	59-61, 73, 74, 79, 87, 98, 104, +	ListImpl	Consistency:2
7	These lines could be simplified by initializing and declaring on the same line		ListImpl	Simplicity
8	Comparison to a Boolean constant, should use: if (!first)?	116	ListImpl	Self-Description:6
9	I prefer the given style, however the checklist indicates that one should use return !first instead of return first= =null	124	ListImpl	Self-Description:6
10	This method is no longer needed since it is not used by the implementation and is not part of the interface.	131	ListImpl	Maintainability?
11	This will throw a null pointer exception if the list is empty, however it is not part of the interface and should not be checked according to the checklist	141	ListImpl	Note
12	No issues found		Node	Note

This code is from the solution to assignment #1.

Added Checklist Item: Maintainability: Misleading comment

Added Checklist Item: Note

There are many indenting problems, however this may be due to the different sized tabs on different machines. I printed the code using a windows based editor.
 
 

Inspector?s Name:Brad Graham

Date Started:22 Feb 2001

Date Completed:23 Feb 2001

Project Name:LinkedTwo-EndedList

Time Report:

Time spent on Familiarization with the Code:10mins

Time spent on Filling in Findings Report:60mins

Time spent on Actual Inspection:60mins

Findings:
 

#	Description	Line #	Class Name	Associated Checklist Item #
1	Line numbering is incremented by 10 instead of 1	all	LinkedTwoEndedList
2	No comments describing instance variables	90-110	LinkedTwoEndedList	Self-Description:3
3	I wouldn?t describe local variables at this point. During maintenance of a method using one of these variables would likely lead to inconsistencies:these comments wouldn?t be updated to match the code modifications.	130-210	LinkedTwoEndedList	Modularity/ maintainability
4	No explanatory comments for method addFront	240	LinkedTwoEndedList	Self-Description:2
5	ListNode not declared as locally as possible	260	LinkedTwoEndedList	Modularity
6	The intention is to check if the list is empty, meaning the size is zero. Checking for size<1 works but is not as clear.It implies that the list could have a negative size.	270	LinkedTwoEndedList	Simplicity:2
7	White space is not needed here	300	LinkedTwoEndedList	Consistency: 1
8	Closing brace should be on next line	400	LinkedTwoEndedList	Consistency: 1
9	No explanatory comments for method addBack	450	LinkedTwoEndedList	Self-Description:2
10	Lines should be indented	460-510,580	LinkedTwoEndedList	Consistency: 1
11	Method body could use some comments	450-590	LinkedTwoEndedList	Self-Description:1
12	No explanatory comments for method getFirst	610	LinkedTwoEndedList	Self-Description:2
13	Indenting	620	LinkedTwoEndedList	Consistency:1
14	No explanatory comments for method getLast	650	LinkedTwoEndedList	Self-Description:2
15	There is no need to create a new ListNode here. You lose all references to it on line 710 when temp3 is set to reference the front.	680	LinkedTwoEndedList	Efficient Storage
16	Lines should be indented	670-800	LinkedTwoEndedList	Consistency:1
17	If the size of the list is null, then front=null and line 740 will throw a null pointer exception	740	LinkedTwoEndedList	Bug
18	No explanatory comments for method removeFirst.It appears the intention is to both return the last object and delete it from the list.	830	LinkedTwoEndedList	Self-Description:2
19	No need to instantiate new ListNode	840	LinkedTwoEndedList	Efficient Storage
20	Method needs comments. The structure of this method is not intuitive. First it checks for list size greater than 1, then checks for size equal to 1, then checks for size zero.	850-990	LinkedTwoEndedList	Simplicity:2
21	This should be front.next.previous=front	890	LinkedTwoEndedList	Bug
22	This line is not needed, the method is supposed to return the contents of the node not the node. Therefore the contents of the temp4.next will not be used.	900	LinkedTwoEndedList	Efficient storage
23	There?s no need to do all of this. Just return null	970-1000	LinkedTwoEndedList	Efficient storage
24	This returns a ListNode not an Object	1010	LinkedTwoEndedList	Bug
25	No where in this method is the size decremented. Two of the cases need to add the line number--	830-1020	LinkedTwoEndedList	Bug
26	No explanatory comments for method removeLast	1040-1190	LinkedTwoEndedList	Self-Description:2
27	Indenting	1050-1180	LinkedTwoEndedList	Consistency:2
28	Poor comment use the word ?size? instead of ?it?	1160	LinkedTwoEndedList	Self-Description:4
29	Number is decreased even if size of list was 0. This line should be in the block with line 1140	1170	LinkedTwoEndedList	Bug
30	If the size of the list was 0 before calling this method, then this line will throw a null pointer exception	1180	LinkedTwoEndedList	Bug
31	No explanatory comments for method size	1210	LinkedTwoEndedList	Self-Description:2
32	Indenting	1220	LinkedTwoEndedList	Consistency:2
	No toString Method		LinkedTwoEndedList	Instrumentation

I add a checklist item for suspected bugs.

We need the TwoEndedList interface in order to check some of the checklist items. For example we don?t know which methods are declared in the interface and should therefore be public in the implementation. Since all methods in the implementation are public you have to assume that they are part of the interface.Some of these public methods (getLast, removeLast) will throw null pointer exceptions if the list is empty.Are these considered anomalies under Anomaly Management:2?

The name of the interface implies implementation detail: TwoEndedList. Is this considered a problem? Further the implication is misleading.Certainly all lists may have two ends, however a TwoEndedList imples both head and tail pointers.The actual implementation only has a head (front) pointer.