The three prime directives should be looked at whenever you try to review a code. The three prime directives are:
- Does the system accomplish a task?
- Can an external user successfully install and use the system?
- Can an external developer successfully understand and enhance the system?
We can use these directives to review how user friendly the code is to both the end user as well as an external developer.
For this project assignment we were to, as a group, create a system that accesses data from the WattDepot server for the Hale Aloha dormitories at the University of Hawaii at Manoa. The system would be a command line interface system that does the following:
- Gets the current power for a tower or a lounge
- Gets the daily energy for a tower or a lounge for a given date
- Gets the total energy for a tower or a lounge from a given date until now
- Ranks the Towers by energy consumption between two given dates
In addition to these commands, we were to design the system in a way where additional commands may be added later.
For this review, I took a look at the hale-aloha-cli-tnt system. It is a command line interface based system that makes use of the WattDepot server to gather information.
So what is WattDepot? If you haven’t read my earlier blogs, then here is a brief summary of WattDepot is. WattDepot is an open source, RESTful web service that collects electricity data (such as power and energy). For more information, check out this link.
Let’s review the system using the Three Prime Directives.
Does the system accomplish a useful task?
The overall functionality of this application meets all the basic things that were required. The project is able to successfully retrieve the current power of a tower/lounge, get the energy for a tower/lounge from a specific date after 22 November to the current date, get the energy usage for a tower between two given dates as long as the were after 22 November, and finally give a sorted order from least to most between two dates as long as the dates were after 22 November. The reason the dates had to be after 22 November is because the server failed around that time and the only accessible data is from after 22 November.
Can an external user successfully install and use the system?
The homepage contains very precise information about what the system does, however the sample input and output is located in a User Guide wiki page. Like mentioned earlier, the User Guide wiki page contains a sample input and output for the system along with simple instructions on running the system. In the download page, the group offers the option to download an executable jar file for those simply wishing to run the application. In addition to this, they also offer an executable jar file along with their source files so that the external users do not need to compile the system first before executing it. The distribution file contains a version number along with a timestamp for external users to monitor any changes and how long ago the changes took place.
In terms of testing, both valid and invalid commands were used. The following are the commands and the type of output it gave: “help”
· “help”
o Gives the proper output.
· “current-power Lehua”
o Gives the proper output of Lehua’s power.
· “current-power lehua”
o Gives a bad syntax error showing that it is case sensitive
· “Current-power Lehua”
o Gives a bad command error showing that the command checker is also case sensitive
· “daily-energy Lehua 2011-11-23”
o Gave the proper output of Lehua’s daily energy
· “daily-energy Lehua 2012-11-23”
o Gives a bad date error.
· “daily-energy Lehua 11-23-11”
o Gives a bad syntax error showing that the date must be properly formatted
· “daily-energy Lehua 11-23-2011”
o Gives a bad syntax error that there is only one way to write the date.
· “daily-energy Lehua 2011-10-21”
o Gives a date error showing that the date should be after 22 November.
· “energy-since Lehua 2011-11-23”
o Gives the proper output of the amount of energy used in Lehua since 23 November
· “energy-since Lehua 2011-12-25”
o Gives a date error
· “rank-towers 2011-11-23 2011-11-25”
o Gives the proper output rating the towers from least to most.
· “rank-towers 2011-11-25 2011-11-23”
o Gives both a date error and a bad syntax error showing that it recognizes that the start date is after the end date.
· “quit”
o quietly closed the application
Can an external developer successfully understand and enhance the system.
The Developer’s Guide wiki page provides clear instructions on how to build the system once the sources are downloaded. Quality assurance is done automatically by running ant –f verify.build.xml that runs checkstyle, PMD, and findBugs. The wiki instructs the user to run this command first before any sort of additional modifications to the code to ensure the original code didn’t have any errors. In terms of coding standards, the wiki page mentions the eclipse format and provides a link to the xml document for users to use. The application wiki mentions that it follows issue driven project management and continuous integration. Continuous integration is a practice in where smaller pieces of code are verified for validation at smaller increments instead of at the end of construction. Often times, each integration is automatically verified (in our case using the “ant” tool) and assists the group find out who, when, and what broke the code if there is a build failure. Issue driven project management is a practice in which the over build is broken down into smaller issues, usually about two days worth of work. For this project, we submitted issues in Google Code and used that service to also monitor and add our issues. A link is provided to the continuous integration server to that is associated with the application. The only thing the Developer’s Guide is missing is how to construct the JavaDoc documentation. The information contained in the Developer’s Guide is very concise and doesn’t contain any useless information.
JavaDoc Review
I was successfully able to create a JavaDoc documentation successfully. After reading through the various class documentations, the documentation shows that the entire application is linked together by the Processor class. The package names are named appropriately where the Main class is contained in a higher-level directory, the command processor is in a subdirectory of the main directory, and the various commands are also in a subdirectory of the main directory.
Build System Review
Using the ant command “build.xml”, I was successfully able to build the system without any errors. The author(s) of each piece of the code are listed in the source code to allow external users to know who wrote each piece of code. In terms of coverage, after running JaCoCo, the project doesn’t have 100% coverage. In some cases such as the Main, Processor, and InvalidArgumentException classes, the coverage is 0% mainly because test cases were not made for those classes. However, the sub-100% coverage is due to the fact that none of the test cases contain a case where the author asserts an invalid statement. The current set of test cases successfully tests the application pretty well for the valid inputs. However, because of the lack of test cases for invalid inputs, there is a slight chance that an external user may create an enhancement that calls throws some random exception and the external developer may have no idea why it is being called.
Coding Standards Review
What I found to be really nice is that each of the command classes were formatted in the same way (because of the interface class). What I found to be very surprising at the same time was hardly any private methods were used. But this is okay since WattDepot provides a lot of the methods already. The only files I had small problems reading was the Main class and the CommandManager class. In the main class the only thing I had issues with was figuring out when the IOException would be thrown. Documentation on what would have thrown it would have been nice. The CommandManager class was beautifully written using some advanced Java techniques that I have not come across before and because of this I had some problems reading and had to take some time to do a bit of research on some of the things. This is was not a problem caused by the author.
Issues Page Review
The issues page associated with this project makes it very clear on who worked on which part of the system. It seems that a different person worked on different classes in the system and because of this, if an external developer had any questions about the system, it would be very easy for them to contact the author(s) that worked on that piece of the system. In the group of three, it seems that two of the developers did more of the work. The other group member worked on a single command class, its test case, the Main class, and the Help class. Most of these classes were fairly short and it is clearly shown in the amount of issues posted by that author. However, I would give him a slight break, as he is an undergraduate student working with a two graduate students.
Continuous Integration Server Review
The Jenkins continuous integration server shows the progress of failed and good builds. Other than at the very beginning of the project between build three and four where the time it took to fix the project was about a day and a half, all failed builds were fixed within an hour. Most were even fixed in as little as 10-15 minutes. As for commits that were associated with issues, less than nine out of ten of them were related to an existing issue. However, the group was pretty close to having nine out of ten commits be associated with issues; approximately eight out of ten.
Conclusion
Based on my review of the hale-aloha-cli-tnt system, I have come to the conclusion that an external developer could successfully understand and possibly enhance its features. Although some of things are slightly advanced for where we are, the enhancements would be relatively simple because the sources are very uniformly organized. The overall system works very well and I was not able to make it crash. The source codes are documented so that most users would be able to understand it, however I would like to have seen comments around some of the exceptions and the things they caught. The group did a very good job implementing it, as well as a very good job working as a group.
No comments:
Post a Comment