MyISERN 1.1 Review

This time we are able to choose the author to review. Therefore, I chose one of the graduate students group to see how they are doing differently from us. The author I picked was Andrew Wong from the Silver group.

Installation Review:

The source code is downloaded successfully. The Google project page also includes the wiki pages for user guide, develop guide, and the release notes. Following the instruction in the user guide, the jar file is successfully created. The verify task ran successfully also, which implies the JUnit, Checkstyle, PMD, and FindBugs are all passed as well. However, when I tried to execute some of the commands to generate the tables, I saw some really chaos and random display instead of a nicely organized table. It took me a while to figure this out: the author is doing the display based on the column numbers, which is obviously a number greater than the normal width of a command prompt window. I tried to resize my command prompt window to 800 columns, and then a nicely structured table finally showed up. The information is mostly horizontally listed, which requires a quite a big width for the window to display them correctly.

I would recommend the author to put this issue into the user guide, since normal users would not resize their command prompt window to verify the display, and they would simply consider the program is not working.

Code format and conventions review:
There are not many code violation can be found. However, for most of the files, the private data members are not documented. (For example, line 46-48 in MyIsernXmlLoader.java) Other than that, the code are well written and easy to read.

Test case review:
Black box perspective: The program does print out the information correctly when there is enough room to display them. However, duplicates and invalid links in the data files are not shown. I would expect to see, for example, at least an error message showing that there is duplicate researcher in my researchers XML file, so that I can go ahead and change it. Other than that, the black box testing seems to be working pretty well.

White box perspective: The Emma report is as follow:
Emma Coverage summary
class: 100% (1/1)
method: 97% (9/10)
block: 100% (497/508)
line: 100% (104/108)
Besides one method that has not been covered, all the other codes are tested. I looked into the Emma report and found out the non-tested method was a method that involves the enum type classes, which Andrew has been mentioned in the discussion. I personally do not know much about enum type, and it seems like the non-tested methods are somehow generated inside an enum class, which is not really inside the program. With the fact that the Java development team is probably responsible for the testing for the enum class, I consider the author did an excellent job in the white box testing.

Break da buggah: There is not really a way to "break" the program, but there are some ways to cause it malfunction, or at least, not functioning as expected. The first way is mentioned before, to execute the program in a normal command prompt window, which it does not print out information correctly. Secondly, the program does not print out error message most of the time for some invalid command calls. For instance, if I invoke the “-describe –all” task by passing something other than “researchers, organizations, collaborations”, the program will simple do nothing.

Summary and Lessons Learned:
Overall, the program is pretty well written. One thing I really like about is the modulization of the program. The author breaks down the program into loader, printer, query engine, checker, and command line parser. Consider this MyISERN project would be last for two semesters, it is definitely necessary to factor the program into different components, which does not interfere each other when one gets changed. I have been thinking to modulize my whole program, and now the Silver group gives me a very good example of doing this. Despite the tremendous amount of work to modulize my current chubby program, I think it is worth the time and effort to do so.