Sunday, November 8, 2009

Systems Review

Over this past weekend I reviewed the Umi and the Umikumakahi systems. Both of the systems gave me a better incite on how to better implement my system. The first system I reviewed was Umi. Overall the system was very solid and I liked the way the implementation was set up.

A. Review the build

I was able to build the system using ant -f verify.build.xml without any problems.

B. Review the system usage:

I was able to execute all of the commands with the exception of the 2.8 command which I called with:

list power generated SIM_WAIAU_8 day 2009-11-15 sampling-interval 30 statistic average

which yielded a command invalid error. But along the lines of invalid commands, I found that the system had very good error handling and that I was unable to crash the system. However one of the downsides was that I was able to execute invalid input on commands that took no arguments. For example I inputted the command: list sources SIM_KAHE_1 which yielded the same results as the command: list command. The same is true for quit (ex. quit test) and the help command.

Also I found that the execution of a false source on the list power generated command caused the system to freeze up and I had to force quit the application. An example of this is:

list power generated SIM_OAHU_GRID_1 timestamp 2009-11-15T12:13:00.000-10:00

C. Review the JavaDocs

Upon reviewing the JavaDocs I found that the system was well documented. All of the class, method and package descriptions were well defined in 1 sentence. Also the effects of the methods were also documented (such as the throwing of errors). However I found that there was no sample code examples and that there was no use of HTML tags in the comment code in the source code.

D. Review the names

After looking at the JavaDocs and the source code itself I found that the system implemented good, logical names for their variables, classes, methods and package names.

E. Review the testing

There was no additional testing besides the tests that were already given with the example system.

F. Review the package design

I found that the name and design of the package of this system made perfect sense.

G. Review the class design

While reviewing the source code I found that the methods accomplished a single well defined task and that the instance variables for the methods as well as the classes and instance variables were all appropriately named. Also the methods were made private as often as possible.

H. Review the method design

I found that the methods of this system were well designed with the exception of the listPowerSample method which was quiet long. Also for this method I found that a block of code stores items in an ArrayList, but this block can be separated from this method and made into its own separate method which would return and ArrayList. Lastly I found that the methods had very low number of side effects as the variables used were usually instantiated in the method itself.

I. Check for common look and feel

I found that the code seem very uniform with no major differences between authors.

-----------------------------------------------------------------------------------------------------------------

This next review is for Umikumakahi. I found that their system had good structure, but other than that it caused me lots of problems, especially since there were no comments in the code besides the JavaDoc comments.

A. Review the build

I was able to build the system with ant -f verify.build.xml without any problems.

B. Review the system usage

I was unable to run the system on my computer. I tired to use Eclipse but the error "Some projects cannot be imported because they already exist in the workspace" occurred which prevented me from importing the project even after I deleted my project from the workspace. In an attempt to test it with another IDE I downloaded and imported the system into NetBeans IDE. However once imported NetBeans reported that the system had 100 errors and these errors prevented me from testing the system. Screen shots of my NetBeans can be found here:

NetBeans IDE
100 Errors

C. Review the JavaDocs

The methods, classes and packagese all have high level descriptions and were all summed up well in one sentence. Also the relationship between classes and packages were well noted. However like the umi system there is no examples of code and the use of HTML tags inside the source codes's JavaDocs comment was not found.

D. Review the names

The names of the classes, packages and methods were well chosen and conform to the standards defined in Java Style chapter 3. However some of the instances of class were misnamed as they were the same as the instance of the object they were instantiating. For example the instance for the Console object was called console.

E. Review the testing

After running ant -f emma.build.xml I found that nearly all the code was accessed with the exception of the WattDepotChartCommand and the CommandLineInterface classes. Also the tests were well thought out and the test results were set to match those sent out by email.

F. Review the package design

This system implemented 2 packages and I found that the packages were logically broken down and that they do a good job separating the commands and the interface itself.

G. Review the class design

Each of the methods in this system do one "well" defined task. I say "well" since each command has its own method so it is easy to tell what method you are looking at, however since there is no comments in the code, I found that often times the method was hard to follow easily. Also there were no private methods used in this system. However the instance variables were all named and used correctly.

H. Review the method design

The method design is good in that it implements only one task. However without any comments in the code I'm not sure if some of the longer methods could be cut down by making a method that could be shared between classes. However the methods have no side effects on the variables since each command has its own class which means the variables are independent of each other.

I. Check for common look and feel

I found that the coding in this system was uniform and there were no discrepancies in the format of coding used between authors.

0 comments:

Post a Comment