Tuesday, November 24, 2009

Wicket Peer Review

For the Wicket review I tested out the Greendepot system.

A. Review the System

I was able to download and build the system without any errors.

B. Review system usage

When I ran tests on the system I noticed that I was able to input a date and have the application recognize the date, however no data would be printed out. Also while testing false values (entering strings or invalid dates) I noticed that all of them resulting in crashing the program. However the interface is simple and self explanatory.

C. Review the JavaDocs

All of the package, class and method summaries provide high-level descriptions with a self contained first sentence. Also the relationships between packages, classes and methods is well described. However the description of the system summary could be better than "a greendepot client". Also all of the JavaDocs conform to the standards put forth in chapter 4.

D. Review the names

In the GreenDepot.java there is a variable called "x". This variable should be renamed into something with meaning. Other than that all of the names in the JavaDocs as well as the actual code conform to the standards set in chapter 3 of Elements of Java Style.

E. Review the testing

There were no tests included with this system besides the HelloWorld test from the example system.

F. Review the package design

The design of the package is good and reflects a logical structure for the system. I found it interesting that the .wicket was left out of the package and instead just went to edu.hawaii.greendepot.

G. Review the class design

Each class is well organized and accomplishes one well defined task. However I think that some of the accessor methods could be set to private as they aren't called on outside of the class that they are contained in.

H. Review the method design

The methods are all short and easy to understand. However in the getCarbonList method of the CarbonCalculator.java I think that the code that creates a timestamp should be in its own method. That way it can be called by other classes and allows for the method to accomplish only one task instead of 2.

I. Check for common look and feel

The look and the feel of the code is consistent. However to improve the coding I would add more comments as there are very few now which causes the code to be a little harder to understand.

J. Review the documentation

The project home pages does a good job of providing the goals for the project as well as what the system accomplishes. However there is no screen dump of the system available.
The UserGuide provides a great step by step method to download and run the system. Just as a small note, I would bold the section titles just so it is easier to find and it doesn't look like one big paragraph.
The DevelopersGuide does a great job of explaining the system and how to extended, but it does not include instructions to build the file, but those can be found in the UserGuide pages.

K. Review the Software ICU data

The ICU seems to be gathering data consistently and reliably. Currently the overall health of the project is good with the only negative aspect being the churn rate. The from the data we can see that the majority of the work took place recently due to the spike in the DevTime and Build fields at the end. Also with the exception of the huge spike in the churn rate we can see that the source code is constantly at a medium to high quality rate.

L. Review the issue management

The issues page was never used during the development of this system.

M. Review continuous integration

There are no long gaps in between commits and with the exception of the first bad build all failed builds are fixed extremely quickly. Also there is a continuous integration job that executes on commit and executes all of the appropriate Ant tasks. Lastly there is a daily build job that executes once a day and executes all of the correct Ant tasks.

Summary

I think that the greendepot system is an ok system. The coding is sound except I think that it could use more comments. Also the lack of outputs, error handling and testing is also a down side to this system. However the based on the ICU and the continuous integration the group seems to be working well and producing good quality code and I think that the next version of this system has the potential to be a well functioning web application.

0 comments:

Post a Comment