code review of wattdepot-cli-umikumaha

This is the wattdepot client interface of team umikumaha (or fourteen) by Dean Kim and Aaron Herres. Below are some issues that I identified.

A. Review the build: there is no problem verify the system with ant -f verify.build.xml

B. Review system usage:
1. The list summary command does not follow the command specification as I've got this error message:

>list summary SIM_KAHE_1
Default list source/sources command

Thus, after reviewed the description from 'Help', and I have to input as below for the summary of SIM_KAHE_1:

>list source SIM_KAHE_1 summary
SubSources: No SubSources found.
Description: Kahe 1 is a HECO plant on Oahu's grid that uses LSFO as its fuel.
Owner: oscar@wattdepot.org
Coordinates: To be looked up later
Properties: (carbonIntensity : 1744), (fuelType : LSFO)
Earliest data: 2009-10-25T00:00:00.000-10:00
Latest data: 2009-11-23T23:45:00.000-10:00
Total data points: 2880

2. The system quit when I inputed the invalid structure for list source.

>list source  SIM_KAHE_1
Exception in thread "main" java.lang.ClassCastException: org.wattdepot.resource.
source.jaxb.SourceIndex cannot be cast to org.wattdepot.resource.source.jaxb.Sou
rce
        at org.wattdepot.client.WattDepotClient.getSource(WattDepotClient.java:1
019)
        at org.wattdepot.cli.ListSourceCommandCli.processSource(ListSourceComman
dCli.java:446)
        at org.wattdepot.cli.ListSourceCommandCli.processCommand(ListSourceComma
ndCli.java:487)
        at org.wattdepot.cli.ListCommandCli.processCommand(ListCommandCli.java:7
6)
        at org.wattdepot.cli.CommandLineInterface.processMainCommand(CommandLine
Interface.java:209)
        at org.wattdepot.cli.CommandLineInterface.processUserInput(CommandLineIn
terface.java:172)
        at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java
:269)
C:\wattdepot-cli-umikumaha-1.1.1104>

3. Mistype for the last token of input command also work. For example,

>list source SIM_KAHE_1 summary2
SubSources: No SubSources found.
Description: Kahe 1 is a HECO plant on Oahu's grid that uses LSFO as its fuel.
Owner: oscar@wattdepot.org
Coordinates: To be looked up later
Properties: (carbonIntensity : 1744), (fuelType : LSFO)
Earliest data: 2009-10-25T00:00:00.000-10:00
Latest data: 2009-11-23T23:45:00.000-10:00
Total data points: 2880

4. The list sensordata {source} day does not work.

>list sensordata SIM_KAHE_1 day 2009-10-30
The input string was invalid.

5. The list sensordata {source} timestamp not work.

>list sensor data SIM_KAHE_1 2009-10-26T12:12:12.000-10:00
Exception in thread "main" java.lang.NullPointerException
        at org.wattdepot.cli.CommandLineInterface.processUserInput(CommandLineIn
terface.java:173)
        at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java
:269)

6. The chart power generated {source} {startday} {endday} sampling-interval {min} file {file} does not work.

>chart power generated SIM_KAHE_1 2009-10-30 2009-11-03 sampling-interval 30 fil
e teste.html
java.io.FileNotFoundException: chart_data_Sun Nov 08 15:33:29 HST 2009.html (The
 filename, directory name, or volume label syntax is incorrect)
        at java.io.FileOutputStream.open(Native Method)
        at java.io.FileOutputStream.<init>(Unknown Source)
        at java.io.FileOutputStream.<init>(Unknown Source)
        at java.io.FileWriter.<init>(Unknown Source)
        at org.wattdepot.cli.ChartPowerCommandCli.generateHtmlOutput(ChartPowerC
ommandCli.java:346)
        at org.wattdepot.cli.ChartPowerCommandCli.processCommand(ChartPowerComma
ndCli.java:394)
        at org.wattdepot.cli.CommandLineInterface.processMainCommand(CommandLine
Interface.java:218)
        at org.wattdepot.cli.CommandLineInterface.processUserInput(CommandLineIn
terface.java:172)
        at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java
:269)
java.io.FileNotFoundException: chart_data_Sun Nov 08 15:35:09 HST 2009.html (The
 filename, directory name, or volume label syntax is incorrect)
        at java.io.FileOutputStream.open(Native Method)
        at java.io.FileOutputStream.<init>(Unknown Source)
        at java.io.FileOutputStream.<init>(Unknown Source)
        at java.io.FileWriter.<init>(Unknown Source)
        at org.wattdepot.cli.ChartPowerCommandCli.generateHtmlOutput(ChartPowerC
ommandCli.java:346)
        at org.wattdepot.cli.ChartPowerCommandCli.processCommand(ChartPowerComma
ndCli.java:394)
        at org.wattdepot.cli.CommandLineInterface.processMainCommand(CommandLine
Interface.java:218)
        at org.wattdepot.cli.CommandLineInterface.processUserInput(CommandLineIn
terface.java:177)
        at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java
:269)
wrote: C:\wattdepot-cli-umikumaha-1.1.1104chart_data_Sun Nov 08 15:35:09 HST 200
9.html

By looking at the location of the output file, I think it's missing the "\" between output file and the source directory.
7. The list power generated {source} timestampe {timestamp} does not follow the command specification. Also, it still does not work when I follow the format from the description in help.

>list powerGenerated SIM_KAHE_1 timestamp 2009-10-26T12:30:00.000-10:00
Exception in thread "main" java.lang.NullPointerException
        at org.wattdepot.cli.CommandLineInterface.processUserInput(CommandLineIn
terface.java:173)
        at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java
:269)

Addition to the commands that I mentioned above, I found that other commands also have similar issue that I don't think it necessary to repeat twice.

C. Review the JavaDocs: There is no problem creating Javadocs with ant-f javadoc.build.xml and also the package and class summary seem to be fine.

D. Review the names: there is no major problem with the naming convention of class or variables in the system.

E. Review the testing:
8. The code coverage result is poor, 49% for method, 25% for block, and 22% for in-line coverage. 9. There also some unused variables that are not clean up.

F. Review the package design:
10. There is only one package in the system that hold the command interface, process the command, and test cases for each command. I think all class commands should be package in another package along with the test class (which eventually will be in a seperate test package when the system growth).

G. Review the class design:
11. To prevent the side-effect when the system growth, I think each command should be in it own class instead of grouping the similar command in one class. For example, list sensordata {source} timestamp and list sensordata {source} day are both in listSensorData class.

H. Review the method design:
12. There is no implementation for list power generated {source} day {day} sampling-interval {min} statistic {max|min|ave} and list power consumed

I. Check for common look and feel: there is no major problem.

After reviewing the wattdepot-cli-umikumaha, I think there are lot of issues with the system that need to fix. I hope my review comment can help both Aaron and Dean to improve their system.