In my
last post, I discussed my experiences in designing a WattDepot team project maintained under continuous integration. The follow-up project involves switching to another team's open-source WattDepot project, updating their documentation, and adding additional functionality to their code base. As a prelude to this, we evaluated their project (and they evaluated ours) according to the
Three Prime Directives of Open Source Software Engineering.
The Three Prime Directives are a set of three guiding principles for open source software development that, when followed, make a system built on an open-source API like
WattDepot useful, easy to use, easy to maintain, and easy to extend. WattDepot, as an API which provides users with "infrastructure for experimentation and development of "Smart Grid" applications," allows users to retrieve stored data on power and energy generation and consumption from a remote server. It was designed with the Three Prime Directives in mind, and it stands to reason that any project based on its tools should reflect the application of those same directives.
Prime Directive #1: "The system successfully accomplishes a useful task."
Any software system must do something which end users will find useful. In the first phase of our WattDepot projects, all systems needed to implement four commands: CurrentPower (the current power being used by a tower or floor lounge in the Hale Aloha residential complex), DailyEnergy (the energy consumed by a tower or lounge on a given day), EnergySince (the energy consumed by a tower or lounge from a given day up to the present), and RankTowers (the four towers' energy consumption from a given start date to a given end date, sorted in ascending order). The project also needed to provide a help function (to tell the user how to use each command) and a quit function to close the program.
The functions were all implemented, and when given input in the expected format, for the most part they returned the expected results. At this stage of the review, though, a few bugs were already apparent:
- current-power should return a value in kW, not kWh.
- rank-towers cannot handle having the same start date and end date; it returns what seems to be a default value for Lokelani and then does not print information for any of the other towers.
- The help command fails to display its message when the .jar file is executed outside of the original distribution directory. This seems to indicate that its file path does not point to a file within the jar (which should be self-contained) but to an external file instead.
This software system fulfills most of the First Prime Directive by implementing most of the useful tasks specified in the original assignment, but its help function is not properly configured to reference other files relative to itself within the jar. Though the .jar works as long as their instructions are followed and the .jar is never removed from the distribution folder, it is not good that the .jar is not self-contained. In case of internal problems like this, a user should have some external documentation, whether stored locally or on the developer site.
Prime Directive #2: "An external user can successfully install and use the system."
Logically, the user of a software system must be able to set up and begin running a system in order to take advantage of the useful task it accomplishes. If the help function does not provide enough information for a user, the next place to go for helpful documentation would be the team's
project hosting site and its
wiki subsection.
*Note to any future readers of this entry: The versions of the wiki pages referenced are as of November 29, 2011. With the exception of the home page (the screenshot of which appears below), the page code for these versions will be available here:
The Home Page
The home page gives a brief introduction to the project's relation to WattDepot and to the Hale Aloha Residential Complex. This is fine, but tells prospective users and developers little about what a WattDepot server does, and nothing about the specific energy consumption information the system is capable of requesting. There is no sample input or output provided on this page; instead, site viewers are referred to the User Guide. The home page's introduction is fine, but it tells viewers very little about what the system is capable of.
The User Guide
The user guide starts by telling the user to "navigate into the directory of the distribution jar." It doesn't tell the user where to download the distribution, or that it needs to be unzipped. A link to the Downloads page is clearly visible in the Google Code website template, but a user guide should not always assume the user knows where on a site the relevant files are stored.
The guide then provides instructions for running the .jar file from the command line, and a useful table of example command syntax. This part of the page is quite accurate. The problem comes in the next table, which provides a list of acceptable names to enter as sources. As I discovered when testing the distribution, the system does not support queries for data from the sub-sources at the [TOWER_NAME]-[FLOOR_NUMBER]-lounge and [TOWER_NAME]-[FLOOR_NUMBER]-telco level; only tower and floor-level lounge names are supported. Thus, "Ilima" and "Ilima-A" through "Ilima-E" will work, but "Ilima-XX-lounge" and "Ilima-XX-telco" (where XX is a two-digit number representing the floor where the lounge is located) will not. While there was no requirement that lounge- and telco- level subsource data requests be supported, the user guide should not misdirect users by saying that the software supports this when it does not.
Distribution
The distribution (version 1.0.2011.11.29.12.22) is clearly numbered according to major release number and date of packaging, and it does contain an executable .jar file, so no issues were present here.
Input Testing
As a general rule, the program handles bad date inputs and incorrect tower and lounge names well, though as mentioned earlier, it does not support querying for lounge- and telco- level subsources as the user guide suggests. Dates that are not in yyyy-mm-dd format are rejected, and dates that are outside calendar ranges (e.g, 2011-13-01, or 2011-02-29) are rejected, and source names that are not the names of towers or floor-level lounges are rejected. (The implementation of this - matching inputs against a hard-coded array of string names - has to be manually edited and is not very extensible, but it works.) The quit function works as well.
However, the system does not do any range-checking for dates in the future.
Enter in a command:energy-since Ilima 2011-12-28
server provided bad xml:org.wattdepot.client.BadXmlException: 400: Range extends beyond sensor data, startTime 2011-12-28T00:00:00.000-10:00, endTime 2011-12-02T04:07:04.920-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Ilima/energy/?startTime=2011-12-28T00:00:00.000-10:00&endTime=2011-12-02T04:07:04.920-10:00
Unable to retrieve energy consumption data
Total energy consumption by Ilima from 2011-12-28 to 2011-12-02 was 0.0 kWh
To its credit, it does give an appropriate error message and returns a default value in addition to the default BadXmlException thrown by the WattDepot server.
A similar problem occurs when requesting daily-energy data for the current date:
Enter in a command:daily-energy Ilima 2011-12-02
server provided bad xml:org.wattdepot.client.BadXmlException: 400: Range extends beyond sensor data, startTime 2011-12-02T00:00:00.000-10:00, endTime 2011-12-03T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Ilima/energy/?startTime=2011-12-02T00:00:00.000-10:00&endTime=2011-12-03T00:00:00.000-10:00
Unable to retrieve energy consumption data
Total energy consumption by Ilima on 2011-12-02 was 0.0 kWh
It seems to be the case that the start and end timestamps used to make the data request are hard-coded to extend from 12:00:00.000 AM midnight on the specified date to 12:00:000.000 AM midnight on the next day. When data requests are made for the present day, the system fails because it requests data outside the available range. This is not a problem for any of the other commands, only for daily-energy.
There was another bug in rank-towers, which gives incomplete output when the start date and the end date are the same:
Enter in a command:rank-towers 2011-12-01 2011-12-01
For the interval 2011-12-1 to 2011-12-1, energy consumption by tower was:
Lokelani 0 kWh
Note that this bug occurs only when the start date and end date are the same, regardless of whether any of the specified dates are the present day or not.
Finally, the program does not do out-of-range checking for dates beyond the server's range of data in the past. This is the case for all of the commands.
Enter in a command:rank-towers 2000-12-01 2011-12-02
Error: Unable to query sources for energy consumption.
Error: Unable to query sources for energy consumption.
Error: Unable to query sources for energy consumption.
Error: Unable to query sources for energy consumption.
For the interval 2000-12-1 to 2011-12-2, energy consumption by tower was:
Lokelani 0 kWh
In all fairness, though, it is impossible to know the start date for any particular server, especially since a server can crash and be restarted without warning. What matters here is that the error is handled with an appropriate message and some kind of default value being returned, and in this respect the handling of this error is correct.
This system partially fulfills the second prime directive by providing error-checking and some wiki documentation for users; however, the wiki documentation is incomplete and sometimes misleading.
Prime Directive #3: "An external developer can successfully understand and enhance the system."
As part of our project requirements, each team's wiki section was supposed to contain a Developer Guide which explained how to build the system from the source code provided in the distribution. The Developer Guide should tell users how to develop for the project, both in a technical and philosophical sense, by specifying environments, console commands, development philosophies, testing requirements, and any other information necessary. Regardless of what it is called, all open source projects should provide information on how to integrate one's system enhancements with the main project by following the same methodology.
Developer Guide
The Developer Guide tells the user to set up Apache Ant, but does not specify a version number (all of our projects were developed under 1.8.2). It then tells the user to run "ant -f verify.build.xml" in the project's root directory. Automated quality assurance through Checkstyle, PMD, and FindBugs is specified as being part of the verify process. However, no coding standards are specified, and the development philosophy is only specified as "issue-driven" without specifically being described as issue-driven project management (which is the only method so far discussed in class). The project's continuous integration server is specified.
The instructions for extending the commands available to the system is quite vague, only telling the user to modify existing files without specifying what parts of the code need to be modified:
"To add additional commands, create a class with the desired functionality that implements the Command interface. Then, modify the CommandParser class to recognize the new command and correctly create in instance of the new class when the appropriate command is entered.
Each Command class and its tests are considered black boxes. The only part of the existing program that should need to be modified (and then, only slightly) when a new Command is added is the CommandParcer class. This modularity should be maintained, as it keeps the system relatively easy to extend."
As far as my teammates and I have been able to work out, commands are added by adding the name used for the command to an array in CommandParser, then instantiating an instance of the command's class within a later set of switch cases, then finally adding a method to call functions from the command's class and pass the appropriate arguments. Normally, even if the external documentation were missing some information, developers should be able to fall back on Javadocs; however, this was not the case.
Javadocs
Javadocs, generated with Java's built-in tool for automatically generating system documentation at compile-time, are a useful source of specific information about methods and instance variables. This system is missing most of its documentation. Methods and classes are tersely documented (e.g., a constructor documented only as "Constructor") and most instance variables are not documented at all. All of the classes that implemented Command also inherited the Javadoc specified in the Command interface, which misspelled "Command" as "Comman." Finally and most obviously, the Index page of the Javadocs says "Implements a Robocode Robot called DaCruzer," a holdover from the Robocode template project on which our Ant configuration files are based.
Information Hiding
Information hiding is the practice of designing a software system such that the specific implementation of design decisions are hidden from external code which references it as much as possible, so that the code can be changed later without affecting the operation of other code. This system has no information hiding of any kind; all instance variables are package-private (Java's default setting) because no public, protected, or private designation was specified, and all the classes are in the same top-level package.
Building The System From Sources
The system builds correctly when "ant -f verify.build.xml" is invoked. and passes Checkstyle, FindBugs, and PMD. It also passes the lone JUnit test for RankTowers, which only has 24% instruction coverage according to Jacoco (which the Developers Guide also does not mention). The other classes have no tests implemented for them, except in the form of text files which import no JUnit assertions and have no @Test methods - and thus would not work as JUnit tests if their extensions were changed to .java. Overall coverage of the project is about 20%, which is usually too low to indicate adequate test coverage of the system.
Source Code
The source code is very sparsely commented as a whole, which did not help when our group was trying to figure out how a developer would go about adding new Commands. In addition, there are many signs of half-finished code, including "//TODO:" comments, missing @parameter Javadoc tags, and commented-out, unused instance variable fields. The strangest thing, as mentioned earlier, are the .txt files labeled TestEnergySince, TestDailyEnergy, and TestCurrentPower. Their names and comments suggest that they were supposed to be unit tests, but including them as text files keeps them from being compiled and run, so it must be concluded that they were not finished in time to make the distribution.
Issues
According to the philosophy of issue-driven project management, a project should be clearly divided into a series of smaller tasks divided evenly among group members, with each task being small enough to be finished in time for the next meeting. The Issues page for this group is misleading; despite the issues with test cases mentioned earlier, all test case-related issues are listed as completed. Each of the three team members (Jesse, Richard, and Micah) was responsible for about a third of the project, with Jesse designing energy-since and current-power, Richard designing the command-parsing class, daily-energy, and help, and Micah designing rank-towers, the Command interface, the command-line interface itself, and quit. Micah seems to have done a little more work than the other team members, and has the only functioning test case.
Continuous Integration
All of the WattDepot projects are verified and re-compiled when the class's continuous integration server, which runs Jenkins. Continuous integration is the idea that a system stored in a repository should continuously be in a verified state, in which all quality assurance and unit tests are passed. Most continuous integration tools provide for automated compilation (either at regular intervals, manually, and/or when changes are made) and testing, as well as notification when a repository commit "breaks" the build by failing a test.
Commits to the hale-aloha-cli-kmj project by date. Dates marked in red are dates for which
one of the projects' required services were down: the Jenkins server was down from November 18
to November 19, and the WattDepot server was down from November 21 to November 24.
If a group is working diligently, commits should occur on a daily basis, and any broken builds should be fixed promptly. If a group is following issue-driven project management, then commit messages should list the issues which a commit addresses, and around 90% of the commits should be non-trivial fixes directly tied to an issue.
The kmj team had no commits for five days (November 10-15), and a look at the Jenkins site reveals that their project remained in a broken state for almost 24 hours after the WattDepot server resumed operation on the November 24, finally experiencing a successful build on November 26. In addition, only 16 of their 26 commits (about 61.5%) are directly tied to an issue; the rest are not.
This team missed most of the requirements for the Third Prime Directive by having low test case coverage, missing Javadocs, a sometimes erratic work schedule, occasionally long times between build fixes, and not labeling commits in accordance with issue-driven project management. As a result of all of these things, it would be difficult for a new developer to understand the system and their development methodology by examining their previous work, and difficult to gauge whether their new code might break the functionality of an untested method.
Conclusion
The hale-aloha-cli-kmj project provides most of the basic functionality required to satisfy the first Prime Directive, and most of the error checking needed to satisfy the second Prime Directive, though some missing information in the wiki keeps it from completely satisfying the second Prime Directive. It is in relation to the third Prime Directive that the state of the project fails to hold up under scrutiny. This is not a criticism of the code itself - which mostly works as expected - so much as it is a criticism of how little it is documented and how much of it is untested. And ultimately, all else being equal, it is reliability, ease of use, and ease of development that will separate an open-source project from others of its kind.
