This is my software review of the WattDepot System Elima branch by Anthony Xu and John Mack. The template I used was provided by Philip Johnson.
A. Review the build
The first part of any review is to verify that the system builds and passes automated quality assurance.
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
If the system builds, the next step is to play with it.
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
-Parent and Description is missing from the output.
list summary {source}
-Command only returns the string “list source summary”
list sensordata {source} timestamp {timestamp}
-Works
list sensordata {source} day {day}
-Output of the command is not formatted correctly. Also doesn’t check the data for the duration of the day.
list power [generated|consumed] {source} timestamp {timestamp}
-“No such command” output when I tried to invoke the command listed in the help menu.
list power [generated|consumed] {source} day {day} sampling-interval {minutes} statistic {max|min|average}
-“No such command” output when I tried to invoke the command listed in the help menu.
list total [carbon|energy] generated {source} day {day} sampling-interval {minutes}
-“No such command” output when I tried to invoke the command listed in the help menu.
chart power [generated|consumed] {source} {startday} {endday} sampling-interval {minutes} file {file}
-Can’t get this command to work properly. I get the following error:
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index: 9, Size: 9
at java.util.ArrayList.RangeCheck(ArrayList.java:547)
at java.util.ArrayList.get(ArrayList.java:322)
at org.wattdepot.cli.command.ChartPowerCommand.doCommand(ChartPowerCommand.java:35)
at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:155)
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 sensordata SIM_OAHU_GRID timestamp 2009-
Unable to create timestamp
Exception in thread "main" java.lang.NullPointerException
at org.wattdepot.client.WattDepotClient.getSensorData(WattDepotClient.java:372)
at org.wattdepot.cli.command.ListSensordataTimestamp.doCommand(ListSensordataTimestamp.java:41)
at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:155)
> list sensordata SIM_KAHE_1 day day
Unable to create timestamp
Exception in thread "main" java.lang.NullPointerException
at org.wattdepot.client.WattDepotClient.getSensorData(WattDepotClient.java:372)
at org.wattdepot.cli.command.ListSensordataDayCommand.doCommand(ListSensordataDayCommand.java:41)
at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:155)
> chart power consumed
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index: 3, Size: 3
at java.util.ArrayList.RangeCheck(ArrayList.java:547)
at java.util.ArrayList.get(ArrayList.java:322)
at org.wattdepot.cli.command.ChartPowerCommand.doCommand(ChartPowerCommand.java:31)
at org.wattdepot.cli.CommandLineInterface.main(CommandLineInterface.java:155)
C. Review the JavaDocs.
Download the system and generate the javadocs (use "ant -f javadoc.build.xml"). Navigate to the build/javadoc folder and click on index.html to display the JavaDocs in a browser. Read through the JavaDocs and assess the following:
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.
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?
-Pretty clear high-level description of the purpose of the package.
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
One of the most important (if not the most important) form of documentation in any software system is the choice of names for program elements, such as packages, classes, methods, instance variables, and parameters. Due to evolution in requirements and design changes, the name originally chosen for a program element may no longer be appropriate or optimal. An important goal of review is to ensure that the names of program elements are well suited to their function. Due to the refactoring capabilities in modern IDEs such as Eclipse, renaming need not be a burden.
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
The JavaDoc review is focussed on whether the system design is correctly explained. In this section, you start to look at whether the system design is itself correct.
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?
-Package structure looks good and organized. This is how the structure of the system is supposed to be like.
G. Review the class design
Examine each class implementation with respect to at least the following issues.
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 not all the class in the system accomplished a single, well defined task. This is because one of the class that does the list source summary command is not complete.
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
Examine each method implementation with respect to at least the following issues.
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