Tuesday, October 9, 2007

16. MyIsernReview


Author reviewed: Chiao-Fen Zielinski

1. Installation Review:
I downloaded the package from the link provided in her blog successfully. Installation went well. There was no instruction on how to execute the program but I was able to invoke "ant jar" from the command prompt to produce a jar file. After that, it wasn't difficult to figure out how to run the program as I could execute the program by "java -jar MyISERN-1-Orange.jar". The tables are produced as desired. Verify passed indicating JUnit, Checkstyle, PMD, and FindBugs tasks all passed too. Her team also achieved 100% Emma coverage. Good job!

2. Code format and conventions review:
There are a few violations in the code:






















File Lines Violation Comments
MyIsernXmlLoader.java 48,49,50 EJS-38 Document all private memberss
MyIsernXmlLoader.java206, 207, 208 EJS-36 Use one-line comments to explain implementation details
TestMyIsernXmlLoader.java51 EJS-7 Extra blank line between methods


3. Test Case Review:
Black box perspective:
Since the purpose of this assignment is to print out the tables based on data given in the 3 xml files, they have fulfilled this requirement. However, they seem to have misinterpreted the data in the colaborations.example.xml as there is only one collaboration item but they split up some data such as "collaborating-organizations" in two separate rows. This would cause confusion as users might mistake the second row as a separate item with missing fields. Since this is a mistake based on misinterpretation of data, even writing test cases could not have spotted that. Other than this, as printing out the data correctly is the only objective, I can't think of other test cases.

White box perspective:
They achieve 100% Emma coverage in all four areas, indicating that all codes has been covered at least once. They also have tests to make sure that the lists are returning the correct value which is nice.

Break da buggah:
The methods to print out the tables are entirely tailored to the data given in this set. If the xml files are revised, such as by adding more items to the collaboration file, the table would fail to print out the new data.

4. Summary and Lessons Learned:
Writing test cases are useful for catching a lot of errors but they still have their limits. In this case, the problem is caused by misinterpretation of data which cannot by spotted by tests. This is when reviewing by another person is especially useful. Through this exercise, I learn about how useful it is to have someone review your code as sometimes you are limited by your perspective and cannot spot some trival minor errors. Also, by comparing their codes with my group's, I learn more about ways to test the main methods as I am quite clueless whenever it comes to writing tests for void methods. On the whole, her team did a good job on this assignment.

No comments: