Monday, October 15, 2007

22.MyISERN-1.1 Review

This time we could choose form a list of names containing classmates and students from Software Engineering 613, which is a master's class, who we want to review. I thought it would be more helpful to review code from a person who is approaching higher eduction. Therefore, I chose to review Lisa Chen's code and it was a brightening experience.

Installation Review
I downloaded the MyISERN Milestone package from this link with no problems at all. Once downloaded, I used the unzip feature and I placed the folder into my test directory C:\test. Then, I looked through the downloaded package for its contents. Luckily, the author has thought about the users, and included a Usage file in plain text format in the doc directory documenting all the commands the user could use on the provided system. Since I already opened the file, I decided to copy some of the commands and try it for workability. However when I pasted the first command from that file into command prompt it gave me this error:

COMMAND: C:\test\myisern-xml-1.0.1007>java -jar myisern-1-purple.jar -listCollaborations -organization University_of_Hawaii

ERROR: Unable to access jarfile myisern-1-purple.jar

Then I tried to build a jar file with this:

COMMAND: ant jar
ERROR: Buildfile: build.xml does not exist! Build failed

The issue with this problem was that, once I imported the unzipped project into my Eclipse environment, an additional folder (myisern-xml-1.0.1007) in my C:\test directory was created. So, I cd under command prompt into the wrong folder because I was using the tab and did not pay careful attention to which directory I change to, since the C:\test directory should only contain the project (myisern-xml-1.0.1015).

Once I figured out this problem, I changed to the proper working directoty (myisern-xml-1.0.1015) which contains all the files needed. This time I was sure that the command in the Usage file would work. However, the same error appeared even after using ant jar.

After a long while, I figured out what the problem was. In the Usage file it says to use this command:

java -jar myisern-1-purple.jar -listCollaborations -organization <uniqueID>

but it should actually be this:

java -jar myisern-1.1-purple.jar -listCollaborations -organization <uniqueID>

Did you catch the error? It took me a while to realize that the jar file is wrong in the provided command. Maybe, I should have written up the command on my own and use tab, then I am sure that I would get the right output. Besides, I am still not sure why I got another directory when I imported the project into Eclipse. The directory contains only the .project file.

Code format and conventions
The code looks really nice! It is properly structured into several classes that contain only methods that also represent the class name. There really aren't any problems regarding the code format and conventions. Also, all variables are properly named.
The only suggestion I would like to give is that when you comment out some code
(I think it is for later use) then I would use this pair /* ... */ instead of the line comment //.

Test case review

White Box
Form a white box perspective the test cases cover a huge amount of the code. The Emma report shows 100% class coverage, 97% method coverage, 98% block coverage, 97% line coverage. These numbers are pretty impressive. Since a lot of effort was put to establish the test cases, I would suggest to add the following test.
In MyIsern.java there is a block of code that is run when the semantics of the XML files are incorrect. You could code your own test semantic XML file and run a test on it. Following the Emma coverage, it seems like in class command.java the method setUniqueId is never used.

Black Box
I looked through all the test java files and it seems like all test cases are well written form a black box perspective. I understand that tests were written for equality, null, and for wrong data on all test java files. Job Well Done!

Break da buggah
I tired for a long time to crash the program but I didn't find yet a good solution. It seems to me that you followed all the requirements correctly and also have a well working system. Here is one way I was able to crash it:

command:
java -jar myisern-1.1-purple.jar -listOrganizations -collaborationLevelGreaterThan 9999999999 it printed
error:
java.lang.NumberFormatException.forInputString(Unknown Source)

But I think that this is lame because an integer can only hold a max value of 2147483647 and I don't think that this program will ever have to hold so many Organizations. Moreover, I got the same error when I inputed a string instead of the required integer for -collaborationLevelGreaterThan.

Summary and Lessons Learned
Reviewing a well written code is very good for improving my coding style in object oriented programming. In this code I saw a lot of different opportunities to improve my coding style. In the next assignment I will concentrate on creating a class or methods that will hold values for me. Basically, I will take advantage of get... and set... . Moreover, I will design class that are easier to enhance.

On the other had, I would like to make you aware that when invoking java -jar yourjar.jar the program gives a list of suggested program usage. That list suggest to use a jar file that is not existing. In other words, it seems like you renamed your jar file in build.xml but did not rename it anywhere else. Moreover, I don't know if it was a requirement but it would be nice to have a command to print all the tables, just like in MyISERN-1.0.

It was nice reviewing your code.

No comments: