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.
Monday, October 15, 2007
22.MyISERN-1.1 Review
Posted by Michal Zielinski at 4:22 PM 0 comments
18.MyISERN-1.1
Overview
This assignment is a Milestone therefore some really useful information for the everyday user as well as for the developer along with some facts on how to improve this wonderful project can be found at this Google wiki page.
Distribution package
Project Hosting
Discussion Group
SVN Group
As always all tasks were completed
What was difficult about this assignment?
First of all, this assignment required more java coding practices than most previous assignments. Actually, the coding was not the problem. However, in my option the time constrain, from getting the "right" requirements, until our group had a clear understanding of the project and could start coding the assignment was a little short. But it was very useful having three people work on the requirements of this assignment, because each of us had different ideas on how to start the assignment. In addition, it was good that we did not had a conflict of choosing which way to go.
The next major problem in this assignment was when using SVN. Somehow, I coded my part of this assignment and wanted to commit the project, but SVN told me that one or more files might be in a conflict state. Since this was the first time dealing with a conflict all of us spend more then two hours on solving it until all of us had the current version on their machine. That is also why we think that our version number is at least at 45.
Finally, since all of us work and each of us has a different working schedule it was little difficult to make everybody "happy" with the meeting time. Nevertheless, as always, for the first couple days of the week we met for about 30 - 45 minutes and discussed potential questions and problems that might appear in this assignment. Furthermore, the last couple of days we met to code the assignment. It was also difficult to concentrate on the assignment since all of us still had many other assignments due this week (for other classes).
What problems were encountered in organizing the group and carrying out the work?
Despite of a couple of different views of the actual interpretation of the assignment requirements, the whole project went very smooth. One reason for that was, because we met every day to talk about the assignment. Another reason was that we made fast decisions on distributing the work and all of us made sure that the job gets done. Moreover, as described above getting this and other projects done, does require to spend some time on it.
What will you do differently during the next Milestone?
I want to promise myself that I will work on the SVN conflict situation. As for this Milestone we spend way to much time getting rid of the conflict. This time could be used to improve our algorithms, however the conflict happened at the last day before the project was due, and it was kind of annoying having to sit there and not to work on the actual implementation (not fun). Concerning time, we actually spend a relative amount of time on this project, so I hope that for the next Milestone my group members will be also as motivated as our group was for this week.
Posted by Michal Zielinski at 6:17 AM 0 comments