Monday, November 9, 2009

Code Review: WattDepot Eono Branch

This is my software review of the WattDepot System Elima branch by Scott Wong and Kelli Sawai. The template I used was provided by Philip Johnson.


A. Review the build

A1. Download the system, build it, and run any automated quality assurance checks (use "ant -f verify.build.xml"). If any errors occur, note these in your review.


-Build Successful


B. Review system usage

B1. Run the system. Exercise as much of its functionality as possible. Does it implement all of the required features?


help

-Works

quit

-Works

list sources

-Works

list summary {source}

-Invoking the command does nothing but returns to the command line interface.

list sensordata {source} timestamp {timestamp}

-Works

list sensordata {source} day {day}

-Doesn’t check the data for the duration of the day.

list power [generated|consumed] {source} timestamp {timestamp}

-Works

list power [generated|consumed] {source} day {day} sampling-interval {minutes} statistic {max|min|average}

-Works

list total [carbon|energy] generated {source} day {day} sampling-interval {minutes}

-Invoking “>list total energy generated SIM_WAIAU_8 day 2009-11-15 sampling-interval 30” results in the following error:

Exception in thread "main" org.wattdepot.client.BadXmlException: 400: Range extends beyond sensor data, startTime 2009-11-15T00:00:00.000-10:00, endTime 2009-11-15T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8182/wattdepot/sources/SIM_WAIAU_8/energy/?startTime=2009-11-15T00:00:00.000-10:00&endTime=2009-11-15T00:00:00.000-10:00&samplingInterval=30

at org.wattdepot.client.WattDepotClient.getEnergy(WattDepotClient.java:566)

at org.wattdepot.client.WattDepotClient.getEnergyValue(WattDepotClient.java:614)

at org.wattdepot.client.WattDepotClient.getEnergyGenerated(WattDepotClient.java:637)

at org.wattdepot.cli.ListCommand.listTotalGenerated(ListCommand.java:362)

at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:109)


chart power [generated|consumed] {source} {startday} {endday} sampling-interval {minutes} file {file}

-Can’t get this command to work properly. Nothing happens when I invoke the command.


B2. Try to break the system by providing it with unexpected input. Can you make the system crash or generate a stack dump? If so, note the input that caused the failure.


>list total energy generated SIM_WAIAU_8 day 2009-11-15 sampling-interval 30

Exception in thread "main" org.wattdepot.client.BadXmlException: 400: Range extends beyond sensor data, startTime 2009-11-15T00:00:00.000-10:00, endTime 2009-11-15T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8182/wattdepot/sources/SIM_WAIAU_8/energy/?startTime=2009-11-15T00:00:00.000-10:00&endTime=2009-11-15T00:00:00.000-10:00&samplingInterval=30

at org.wattdepot.client.WattDepotClient.getEnergy(WattDepotClient.java:566)

at org.wattdepot.client.WattDepotClient.getEnergyValue(WattDepotClient.java:614)

at org.wattdepot.client.WattDepotClient.getEnergyGenerated(WattDepotClient.java:637)

at org.wattdepot.cli.ListCommand.listTotalGenerated(ListCommand.java:362)

at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:109)

>list sensordata SIM_OAHU_GRID timestamp break

should never happen.

Exception in thread "main" java.lang.NullPointerException

at org.wattdepot.client.WattDepotClient.getSensorData(WattDepotClient.java:372)

at org.wattdepot.cli.ListCommand.listSensorDataTimestamp(ListCommand.java:320)

at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:65)

>list sensordata test day test

should never happen.

Exception in thread "main" java.lang.NullPointerException

at org.wattdepot.client.WattDepotClient.getSensorData(WattDepotClient.java:372)

at org.wattdepot.cli.ListCommand.listSensorDataDay(ListCommand.java:92)

at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:77)


C. Review the JavaDocs.

C1. Does the System Summary (provided in an overview.html file) provide an high-level description of the purpose of the system? Does it explain how each of the packages in the system relate to each other? Is the first sentence self-contained?


-The system summary provides a high-level description of the purpose of the system. Doesn’t really explain how the packages in the system relate to each other because only one package exist.


C2. Do the Package Summaries (provided in package.html files) provide a high-level description of the purpose of the package? Do they explain how the classes in the package related to each other? Is the first sentence self-contained?


-Main package summary refers to an example of WattDepot interaction.


C3. Do the Class Summaries (provided at the top of each .java file) provide a high-level description of the purpose of the class? Does it provide sample code for clients of the class, if useful? Is the first sentence self-contained?


-Accurate high-level description of the purpose of the class. Doesn’t provide sample code for clients of the class.


C4. Do the Method Summaries (provided before each method) explain, from a client-perspective, what the method does? Do they avoid giving away internal implementation details that could change? Do they document any side-effects of the method invocation? (Note that you can click on the method name to see the source code for the method, which is helpful to assessing the correctness and quality of the javadoc.)


-The method summaries does a pretty good job on explaining what the method does. They were able to do it without giving away internal implementation details.


D. Review the names

D1. Do another pass through the JavaDocs, this time concentrating on the names of packages, classes, and methods. Are these names well chosen? Do they conform to the best practices in Elements of Java Style, Chapter 3? Can you propose better names?


-Naming scheme looks fine.


D2. Once you have reviewed the names displayed in the JavaDocs, review the source code for internal names in the same way.


-Source code internal names looks fine also.


E. Review the testing.


The system should provide a useful set of test cases.


-Only test present is the example test provided by the instructor.


F. Review the package design

F1. Consider the set of packages in the system. Does this reflect a logical structure for the program? Are the contents of each package related to each other, or do some package contain classes with widely divergent function? Can you think of a better package-level structure for the system?


-System only contains one package. Maybe create a separate package for commands class, test cases, etc.


G. Review the class design

G1. Examine its internal structure in terms of its instance variables and methods. Does the class accomplish a single, well-defined task? If not, suggest how it could be divided into two or more classes.


-After going through the instance variables and methods of the system, I conclude that the classes in the system could be divided into separate classes instead of one big class that does many things. This will make the code easier to read and makes creating test cases easier.


G2. Are the set of instance variables appropriate for this class? If not, suggest a better way to organize its internal state.


-Looks fine.


G3. Does the class present a useful, but minimal interface to clients? In other words, are methods made private whenever possible? If not, which methods should be made private in order to improve the quality of the class interface to its clients?


-No private methods observed in the system.


H. Review the method design

H1. Does the method accomplish a single thing? If not, suggest how to divide it into two or more methods.


-Each methods seems fine in accomplishing a single thing.


H2. Is the method simple and easy to understand? Is it overly long? Does it have an overly complicated internal structure (branching and looping)? If so, suggest how to refactor it into a more simple design.


-The methods looks fine. They are easy to follow and understand.


H3. Does the method have a large number of side-effects? (Side effects are when the result of the method's operation is not reflected purely in its return value. Methods have side-effects when they alter the external environment through changing instance variables or other system state. All "void" methods express the results of their computation purely through side-effect.) In general, systems in which most methods have few or zero side-effects are easier to test, understand, and enhance. If a method has a large number of side-effects, try to think about ways to reduce them. (Note that this may involve a major redesign of the system in some cases.)


-No side-effects observed.


I. Check for common look and feel


I1. Is the code implemented consistently throughout the system, or do different sections look like they were implemented by different people? If so, provide examples of places with inconsistencies.


-The code seems to be consistent throughout the system. It doesn’t look like two people implemented the code.

0 comments:

Post a Comment