Wednesday, December 14, 2011

Uncharted Waters: Extending a Foreign System

Once again, it is time for team JCEV to tackle another CLI project for the Hale Aloha towers.  However, this time we did not design the system from scratch.  Instead, we took control of the hale-aloha-cli-kmj project that I previously reviewed and extended it by adding three new commands and updating the documentation.  So how did it turn out?  Well, as per usual I shall evaluate this new endeavor in the context of the three prime directives of open source software.

Prime Directive #1: Does the system accomplish a useful task?
To accomplish this prime directive, the system should fully implement the three commands specified.  The first command is set-baseline which records the energy used by a tower or lounge over a specified day in one hour periods and it should default to yesterday if no date is given.  In this case, the system successfully retrieves this data and stores the information in a Baseline object for later use in both cases.

set-baseline command with optional date argument.
set-baseline without the optional date argument.
The second command, monitor-power, should periodically retrieve the current power consumption of the given tower at the given interval.  The interval argument is optional and the interval between checks should be 10 seconds if not specified otherwise.  In addition, the command should stop execution when the user presses a key like enter.  This version of the system contains of the functionality as it periodically prints the results as expected.  The only small issues are that the command will not stop unless the enter button is pushed (i.e. the s and the a in the image below did not stop the command's execution) and that terminating execution causes an invalid command error.  These do not really affect the functionality though and hence this command is in working order.

The monitor-power command in action.
The third and final command is monitor-goal.  This command checks the current power consumption of the given tower or lounge and compares it to the baseline which must be set before running this command.  If the current power consumption has been reduced by the specified goal percentage or more, than it prints the goal has been met.  Otherwise, it indicates that the goal was not met.  The monitor-goal command does this periodically like the monitor-power command and should stop execution when the user presses a button like enter.  This implementation of monitor-goal suffers from the same problem as the monitor-power command as it requires the user to press enter to exit the command, but once the user presses enter, the command exits as expected.  As a result, all three commands work to accomplish a useful task and this system fulfills the first prime directive.

The monitor-goal command successfully executing after the baseline has been set.

Prime Directive #2: Can an external user successfully install and use the system?
I believe that this system fulfills this prime directive as well.  The original project was a little lacking in the user documentation due to a sparse home page and a few key errors in the UserGuide, but these have been fixed in the latest version.  The home page now sports many screen shots of example executions that users can reference and the new UserGuide removes some of the misleading instructions that afflicted the original guide (i.e. removed the unsupported sources that were previously listed).  The system is also very easy to install as there is a clearly labeled distribution to be downloaded on the website which contains an executable jar file that can be run without any compilation.  Consequently, I believe that the updated documentation should make this system easy to install and use, even for an external user.

Prime Directive #3: Can an external developer successfully understand and enhance the system?
Essentially, this entire exercise was a test of the third prime directive as my team (JCEV) had to adopt this code-base, understand how it worked, and extend it.  While we were successful in doing so, it was not an ideal experience as we had to figure out how to extend the existing system on our own.  The DevelopersGuide gave a general overview as to how the system worked, but the specifics of what changes were needed when adding a new command were nowhere to be found.  As a result, we had to skim over the code in the CommandParser class to figure out exactly what these changes were before we could add our new commands.  Of course, this was a pain and we decided to pardon any future developers from this experience by listing the required changes in the updated DevelopersGuide.  The DevelopersGuide was also updated with the coding standards that should be used in the project and some instructions detailing how to generate JavaDocs to further assist any future developers.  These changes should make it much easier for an external developer to successfully understand and enhance the hale-aloha-cli-kmj system and thus I think that it now fulfills the third and final prime directive.

Conclusion
Overall, this was an interesting experience in software engineering.  While we managed to implement all of the functionality and produced what I would consider high-quality software, it was not without its issues.  These issues mostly stemmed from trying to extend an unknown code-base with rather poor documentation as a good amount of brain power and time was spent analyzing the existing source code to figure out how to fit our new features in.  This was probably the first time in my programming experience that I had to learn what someone else's code did by reading it and it definitely emphasized the importance of good documentation.  Finally, this experience also made me realize just how essential good group members are.  When everyone did their part, the workload becomes much more bearable and the project can progress smoothly.  We did not finish with a lot of extra time like our own CLI project, but we did have enough time to double check everything to give ourselves the piece of mind that we did our best and that the product is up to our standards.  As a result, I would like to thank Eldon Visitacion and Jordan Takayama for their hard work throughout this semester.  It has been a pleasure working with them in group JCEV and I am looking forward to working with quality individuals like them in the future.

Friday, December 2, 2011

A Technical Review

Collaboration is the name of the game when it comes to Software Engineering and an important part of the collaboration process is performing technical reviews on other developers' code.  Here we shall evaluate a group's Hale Aloha CLI project where they attempted to create an open source Java command line interface that would allow users to query a server for various energy / power related information.  This was to be done using the WattDepot library which simplifies the task of connecting to the server and retrieving the data so that the developer can focus on processing that data.  As a result, we will not focus so much on the code itself, but take a look at how "good" this project is by comparing it to the three prime directives of open source software.

Prime Directive #1: Does the system accomplish a useful task?
The first prime directive is pretty self explanatory, does the system do what it is supposed to do?  Here I tested the various commands that the system was supposed to implement and checked if they worked as I expected them to.  This initial release of the system is expected to implement 6 commands: help (return a list of commands), quit (exit the CLI), current-power (returns the current power usage of given tower/lounge in kW), daily-energy (returns energy used by the given tower/lounge on given date), energy-since (returns energy used by given tower/lounge from given date to now), and rank-towers (rank towers from least to most energy used on given interval of days).  All six of these commands are present in the system and for the most part are functional, but there are also a few problems.

The first major problem is that the commands do not work with all of the advertised source values.  The UserGuide lists lounge and telco sources (i.e. "Lokelani-04-telco") as valid, yet the system rejects them claiming that they are not valid.  It works fine with all of the tower  (i.e. "Lokelani") and lounge (i.e. "Lokelani-A") aggregates though.

The system failing to recognize a telco source as a valid source.

Another problem is that help command breaks if the jar file is moved to a different directory.  The current implementation of the help command references an external text file to retrieve its data with a hard-coded path and if the jar is moved outside of the distribution directory, the help command fails to work.  This is a problem since I think that the executable jar file should be self-contained and fully usable from anywhere in the file system.

The help command failing when the jar file is moved.

Yet another issue is that invoking the daily-energy command with today's date does not work.  It does not work as the command tries to retrieve energy data between the start of given date (12:00 am today) and the start of the next day (12:00 am tomorrow) which is an invalid range since the end of the range is in the future.  While the project specifications does not explicitly state whether today is a valid input for daily-energy or not, I expected the command to retrieve data from the start of today until the time of the latest recorded sensor data instead.

The daily-energy command failing to work when given today's date.

Finally, rank-towers does not work when it is given the same day as the start and end dates.  Instead, it will print out just one line for the Lokelani tower with a value of 0.  This is not good as giving the same date should either be considered a valid output and print results for all four towers or be flagged as invalid and raise an error message to inform the user.  Thus, I believe that the current behavior of the rank-towers command when given the same day as the start and end dates is unacceptable as it does not indicate if that input is valid or not and prints an incomplete output.

The strange output from rank-towers when using the same day as both the start and end dates.

Overall, most of the functionality is there.  Outside of these cases, all of the expected commands are present and work as expected by printing out results when given valid inputs and printing error messages when given invalid inputs.  Consequently, I believe that this system somewhat fulfills the first prime directive as long as the user is careful about which inputs is used and does not move the jar file.

Prime Directive #2:  Can an external user successfully install and use the system?
This directive tests the documentation of the project from the view of an external user.  Unfortunately, the home page does not tell the user much about the system and lacks any sample input and output.  Furthermore, the UserGuide does not tell the user where to download the distribution, how to "install" the system (i.e. extract it from the zip archive), and the command to invoke the executable jar file is wrong (the name of the jar is "hale-aloha-cli-kmj.jar" not "hale-aloha-cli.jar").  On the other hand, the UserGuide does do a fairly good job at explaining which commands and arguments can be used (thought there is that error as mentioned in the the previous prime directive), the distribution is labeled with a version number to distinguish it from past versions, and the distribution does contain an executable jar that does not require any building or compilation.  However, the aforementioned problems are rather major so the system only partially fulfills this prime directive.

Prime Directive #3: Can an external developer successfully understand and enhance the system?
The third and final prime directive once again tests the project's documentation, but from a developer's view.  A major document for this directive is the DevelopersGuide which should detail how an external developer can build the system and how to extend it.  While it does detail how to build the system using Ant, it does not tell the developer how to generate the system's JavaDocs.  It also does not mention that new functionality must be accompanied by new JUnit test cases as it only mentions that additions must pass CheckStyle, PMD, and FindBugs.  In addition, the DevelopersGuide makes no mention any standards that are being followed.  The instructions for extending the system are rather vague too as it does not explain exactly what to modify in the CommandParser class to add a new command and makes does not describe how to add any new commands to the help command.  Finally, the guide mentions that the project is being managed in an issue driven fashion, but does not explain exactly what that means for developers (i.e. make a new issue for every new command / functionality).

Outside of the DevelopersGuide, JavaDocs and source code comments can help external developers understand the system.  Overall, the JavaDocs do a decent job of explaining what the functions do and what the expected arguments are even though there are a few misspellings (i.e. Comman's instead of Command's in the Command interface's printResult method's comment), fields are not commented, and the overview.html does not fit match the project.  Conversely, the source code comments are a little sketchy.  Some source files like EnergySince.java and HaleAlohaCli.java include inline comments to tell readers what the code is doing at a glance while others like CommandParser.java have no such comments, leaving external developers to figure out the code on their own.  Another problem is that most of the classes do not support information hiding.  Ideally, all fields in a class should be private and only be accessible through getter and setter methods, but several classes like EnergySince and HelpCommand declare their fields without the private keyword.  This makes them protected by default and allows any class within the package to manipulate the fields which gives external developers the ability to put objects into possibly invalid states and unknowingly break the system.  All in all, this project does not satisfy the third and final prime directive very well as it fails to explain all of the expectations that are in place when extending the system, does not consistently support information hiding, and leaves a few critical things up to the developer to figure out on their own.

Other Expectations
In addition to fulfilling the three prime directives of open source software, there some additional requirements that the system should meet.  One such requirement is that the system should be well tested so that it is easy to find out if enhancements broke pre-existing pieces of the system.  To be well tested, the developers should have created JUnit test cases to check most of the functionality of the system while executing a majority of the code for good code coverage.  Unfortunately, this system fails to do this as it includes only one real JUnit test case.  While there are other "test cases," they do not use JUnit and have a .txt extension so they are not automatically run by Ant when the system is verified.   Additionally, the one JUnit test case that is present only tests three out of the five methods in the class that it tests.  The result is a barely system that has about 18% of its code tested.  Hence, this system does not meet that requirement as there are virtually no tests to determine if any new additions broke existing components of the system or not.

Another expectation was the use of issue driven project management (IDPM) to evenly distribute the work between group members.  In IDPM, members meet every few days to divide the current tasks into small work units called issues which should be tracked on the Issues page of the project.  In addition, most commits to the systems should have an associated issue in its commit log to help explain why that change was made.  By looking at the Issues page it is quite clear who was responsible for what and it shows that the work load was not exactly even as Micah had 7 issues to work on while Richard and Jesse only had 4 issues.  They did not do a very good job at linking their commits to their issues either as only 19 out of 26 commits (~73%) are linked with an issue.  Some of the changes were minor though and can be excused, but that would still put them below the desired 90% of commits associated with an issue.  Therefore, despite their efforts, this group did not quite make the best use out of IDPM as they worked on this project.

The final expectation is the use of continuous integration.  Continuous integration tools attempt periodically to build and test the system (i.e. after every commit) to ensure that the system stays in an acceptable state.  If it fails to pass these tests, the project members are alerted so that they can find and fix the problem.  This project uses a Jenkins server which can be found here as mentioned in the project's DevelopersGuide.  By looking at the Jenkins server, we can get an idea of how development of the project progressed.  There are a total of 9 failed builds, but 5 of them were a result of the server used by the system being down and was out of the group's control.  The other 4 failed builds were promptly fixed though with an interval of five hours being the longest the system was in a failed state which is not too bad considering that this interval was in the middle of the day when the developers were probably busy with classes.  The rate of commits seems rather good too.  Between 11/10 and 11/15 and between 11/23 and 11/26 there were no commits made which implies that no significant work was completed in these time frames.  This equates to about one week of lost time which is definitely concerning, but the rate otherwise seemed quite consistent with a couple of commits every day or two.  Consequently, it looks like this project made good use out of the available continuous integration tools.

The build history of the project as found on the Jenkins server.

Conclusion
Overall, this project has some problems.  It fails to completely satisfy all three prime directives of open source software due to incomplete documentation and weak testing and it also seems to fall short of some of the other expectations from this project as the weak testing lead to poor test coverage and they were not quite able to use issue driven project management optimally.  Despite these issues, their use of continuous integration was good and the overall system works in most cases.  In conclusion, this project could use a few tweaks, but reviewing it as an outsider has been an interesting and enlightening look into the software development processes of other developers.