MyISERN Review

I was assigned to review Jung Kim’s code this time, but by the time I was writing this review, I could not find the entry from his blog for the MyISERN assignment. Therefore, I went to his team’s Google project page to download the source code.

Installation Review:

The source code is downloaded successfully. There is no wiki page for the user guide or something like that, but after all this was not required in the previous assignment. I had no problem at all while trying to import the project into eclipse. I invoked the verify test in the command prompt, and it ran successfully. This implied the Checkstyle, PMD, FindBugs, and JUnit tests should all be passed.

Since the first MyISERN assignment is relatively simple, there is not much code added in the program. I can easily understand the code.

For the program execution, I invoked “ant jar” and it creates a jar without any problem. Then I invoke the jar file by “java –jar” command. The tables were printed out correctly. Overall, the program does what it supposes to do, and it functions correctly.

Code format and conventions review:

There are a few violations in the code:

File	Lines	Violation	Comment
MyIsernXmlLoader	152, 169, 196, *	EJS #7	Unnecessary blank line within a method
MyIsernXmlLoader	142, 159, 194, *	EJS #9	Pick more meaningful variable name or give it a comment
MyIsernXmlLoader	191	EJS #27	Name for collection should be plural

Test case review:

Black box perspective: The test does print out the table as expected. Other than that, the number of researchers, organizations, and collaboration are also tested, but that is given from the initial files. However, printing out the table is the only objective of the assignment, and the data are all given, there is not much black box testing can be done.

White box perspective: The Emma report is as follow:
Emma Coverage summary
class: 100% (1/1)
method: 90% (9/10)
block: 98% (497/508)
line: 96% (104/108)
Although the coverage is not 100%, it does not mean the testing is not sufficient from our experience of code coverage. By inspecting the testing case, I found out the reason for the missing coverage is that the main method has not been tested. However, since main method is “evil”, and JUnit is almost useless when it comes to testing of the standard output, I consider the author has done a fairly good job on the testing. Also, there is one line of error checking code is not covered. As mentioned above, the data for the assignment is given, thus an error input cannot be produce and the error checking will never be executed.

Break da buggah: I could break the program by running it outside its original folder, because the XML data files are defined such that they are located within the original folder. However, this seems a little bit too extreme to me because I think only a programmer that knows the internal structure of the program would know this could cause the program to fail.

Summary and Lessons Learned:
Overall, the program is pretty well written. It functions correctly and safely. One lesson I learned is that when the data is given, there is still no guarantee that the program will not go wrong. Furthermore, things tend to change a lot in real life, and errors usually emerge during the change. Another lessoned I learned is the importance of documentation. Not only the meaningful comment in the code is necessary, but also a user guide or manual is very helpful.