16.MyIsernReview

As always the goal of reviewing peer's code is to improve the ability to read, write, and test Java systems. Today, I will review Kevin's code who is a graduate student taking Software Engineering 613 at the University of Hawaii.

Initial Problem
I was assigned to review Kevin's code. When visiting his Blog entry for MyISERN-1.0, I did not find and links pointing to the distribution file. Technically, I would be done now, since I was not able to download any project to review. However, I like reviewing someone's code because most of times I learn more about the Java programming language. Therefore, I went through the list of names trying to figure out if his project partner had a pointer to the project distribution. I was glad that he had, because it saved my contacting Kevin by email and it also saved him some stress editing his Blog entry.

Installation Review
I have downloaded Kevin's project from this link. The download went smooth and I was able to unzip the distribution into my local directory. I also did not have any complication when importing the unzipped distribution into Eclipse. Right after importing the project into Eclipse, I decided to open the command prompt and try executing his code using this command: java -jar .... however, as it seemed there was no jar file present. Therefore, I tried to create a jar file using the ant command ant jar. The build was successful and a jar file was established. Now I was able to invoke java -jar myisern.jar and it printed the desired three tables within the prompt. Although, the tables could be formatted nicer, they seemed to have the correct values. (Tables look better in Eclipse) Running the command ant -f verify.build.xml did not return any errors and Emma coverage was at 100%. However, a 100% Emma coverage doesn't indicate that the system was properly tested which I will find out in the "Break da buggah" section of this assignment.

Code format and conventions
This section was pretty well covered. I could only find a few violations which are presented in the table below.

FILE	LINES	VIOLATION	COMMENTS
TestMyIsernTable.java	6	ICS-SE-Java-2	Do not use wild card "*" in import statements
TestMyIsernTable.java	1	EJS 41	Provide a summary discription (Empty)
MyIsernTable.java	82	EJS 32	Write documentation for those who must use your code and those who must maintain it
MyIsernXmlLoader.java	123	EJS 1	Adhere to the style of the original (no space)
TestMyIsernXmlLoader.java	18	EJS 1	Adhere to the style of the original (@Test)

Test case review

Black box perspective

There are tests that test if the correct values are read from the XML files. The method testGetResearchersTable() tests at least one field from each researcher. However the field picture-link and Bio-statement are never tested. Therefore, I would suggest to test these two values at least in one researcher. In the second test method testGetOrganizationsTable(), I would suggest to also write a test for Affiliated-Researchers. Since for one researcher this value is null and for the other not. Checking this test case, made me wonder if the table output is correct. That is why I ran the program. I figured out that it doesn't even print the Affiliated-Researchers. Then I found out that for other tests it is similar. Wherever I thought there is a test case missing, it did not print the values to standard output. Now, I am thinking if this was done with purpose, because otherwise the content couldn't fit onto the screen the way the table was formatted, or were the values left off unintentionally.

White box perspective

The testing is well done from the white box perspective. I ran Emma on this program and it returned a 100% coverage for all areas. Job well done! But, I suggest not to rely on that result. After knowing that the code is covered 100%, I would advise to close the coverage report and look at the test cases from a block box perspective again. Make some logical decisions whether to test each important part or just do testing for certain values.

Break da buggah

When running this program it does not return all the values contained in the example XML files. Does this mean that it returns incorrect values? Other then that, even if I run it with additional arguments it still prints the tables. On the one had it is good because this doesn't mean that the program crashed, but on the other hand it would be useful to the user if a statement would be printed informing that additional arguments are not supported at this time.

Conclusion

I believe that for this assignment we supposed to print ALL the values contained in the example XML files. However after reviewing Kevin's code, it made me wonder, because that code did not print all of the values, at least I didn't see them on the console. Especially, because the program seemed to work and look right. Certain values were missing in the output. Now, imagine if this would be distributed to the customers and they could not find the needed values in the just so expensive bought software. Seems like I learned that designing logical test cases and checking the output is very crucial to the correctness of a software output.