Wednesday, December 14, 2011

Hale Aloha CLI v2: hale-aloha-cli-cycuc

Taking over a project that another group started is somewhat like trying to bathe a cat: there is a lot of blood and screaming involved. This was very much the case in the work that Team Teams has done on the project that Team cycuc created, though fortunately without the blood. We began our additions to hale-aloha-cli-cycuc soon after writing our technical review of the project. While we did discuss whether or not a developer could “successfully understand and enhance the system,” all of that was based on examining standards and documentation and tools, not actually attempting to work on the project ourselves. Without doing so, it is very difficult for us to give an accurate answer as to how easily development would progress. Thus, this development is simply a continuation of our review of Team cycuc.

Team Teams is still comprised of the same members: Jayson Gamiao, Jason Yeo, and me. Team Teams has implemented some additional commands into Hale Aloha CLI, which are as follows:

set-baseline: stores baseline values of power consumption for a given source over the course of a given day. These values are then used for the next command,
monitor-goal: uses the baselines from set-baseline in conjunction with the current power consumption to determine if the given source has met a given power reduction goal. The current time, the current power consumption value, and whether or not the source met the power reduction goal are printed at given time intervals until the system receives a carriage return.
monitor-power: prints out the current power consumption of a given source at given time intervals until the system receives a carriage return.

These commands expand the capabilities of Hale Aloha CLI in a few ways. First, none of the prior commands truly depended on stored data. While data was saved before—the energy consumption of the towers for rank-towers, for example—it was always within a particular command, and once the command was complete that data was lost. In contrast, the set-baseline command explicitly stores data for later retrieval, and the monitor-goal command requires access to that stored data.

Another addition, related to the first, is the need for communication between different commands. The set-baseline must store data in a manner allowing monitor-goal to access the data.

The most complex of these new features though is the monitoring aspect. Both monitor-goal and monitor-power must wait until the user provides some input. In this, Team Teams did not quite achieve our objectives: though the original specifications were to end the loop upon any keystroke from the user, the actual system only checks for carriage returns. Another deficit in the system is that the data printed to the screen is not necessarily real-time: the power values are the latest ones obtained from the server, and thus are not necessarily at the intervals that the user provides. In some cases, output on these monitoring commands will "stutter," producing pairs of output with the same timestamp and values.

Team Teams generally worked on this in the same manner as we had on hale-aloha-cli-teams. Work was distributed in much the same way; we all generally had the same responsibilities as on our original project. As our coding standards and tools were the same as those that Team cycuc used, those naturally remained the same as well.

However, we were also much busier while working on this project due to other commitments. Thus, rather than consistent updates throughout the project timeline one will see several spikes in activity when we had time to work on the project. Unfortunately, this also led to us having to rush to finish the project on time and as a result most of our commits were on the day before our deadline. We were certainly working on the project consistently, but this did not always translate to commits.

We also tended to ask each other for assistance more often than we did on the original project. This might have been due to increasing familiarity with one another, particularly since we all suddenly had at least one thing in common: a distinct hatred for the way that hale-aloha-cli-cycuc was structured. On the other hand, the unknown structure of the system made some overlap in our areas of responsibility inevitable. For example, while I handled the set-baseline and monitor-goal commands, I had to use the HaleAlohaClientUI class, nominally the responsibility of Jayson, to transfer data from one to the other. In turn, while Jayson normally handled input and output, hale-aloha-cli-cycuc has the output in the Command implementations, so Jayson had to go into my command classes to work on the output. Regardless of our rationales, we did work together a lot more and cover for one another. As a result, while the Issues page for hale-aloha-cli-cycuc may give a general idea of who did what, it vastly underestimates the amount of work that each of us completed. I know that I wrote a little bit of almost every file that we added or modified for this project, and I am fairly certain that the rest of my group members did the same.

Overall, the quality of our work on hale-aloha-cli-cycuc is not too bad. Obviously, the code is nowhere near as good as it would have been had we continued to work on our original code base. However, the work that we have done certainly does perform as expected, and we were able to resolve at least some of the issues that we brought up in our technical review.

Going back to the Three Prime Directives that we used in our technical review:

Review Question 1: Does the system accomplish a useful task?
Yes, the system does accomplish what it should. All of the commands are implemented and run properly.

Review Question 2: Can an external user successfully install and use the system?
Yes, a user can easily download the system from the Downloads section and use the system with the .jar file available in the root directory. The UserGuide page in the wiki on the project site now has instructions on the procedure for installation and running the .jar file. The project site remains mostly unchanged and thus provides the same information as before. Unfortunately, Team Teams was only provided committer status, and while that has generally been enough it does have its limitations. One such limitation is the inability to change much of the project site.

Output is now slightly more organized than before. One of the complaints that we had before concerned the units for the output values, which were incorrect or not present. These have been standardized as kilowatts for power and kilowatt-hours for energy.

Review Question 3: Can an external developer successfully understand and enhance the system?
The DeveloperGuide page on the project wiki remains unchanged, and thus continues to provide a rudimentary explanation of how developers can build and develop the system.

Each class and method has a description in JavaDocs. However, Team Teams did not change any of the JavaDocs descriptions that were already in the code, save to add descriptions where none existed; as a result, some of the descriptions may not be ideal.

Building the system is difficult to accomplish due to timeouts; this may be due to my individual machine though. The system builds perfectly fine on Jenkins.
Jacoco coverage remains universally horrible. Again, this is due in part to timeouts. I admit though that the Baseline class, which I created, does not have a test for anything; it is only tested indirectly through its use in the set-baseline and monitor-goal commands. This does indicate though that about half of the methods in the Baseline class are unnecessary, since they are never called in a practical situation.

Testing as a whole remains a bit weak. Team Teams was unable to effectively test the code due to the structure of the hale-aloha-cli-cycuc project. The Command interface has a void method printResults. Since printResults does not return anything, there is no return value to test. Though it is possible to get around this through storing the most recent value as a field and creating a getter method for that field, this is not very efficient.

Most of the coding standards violations mentioned in the technical review have been resolved. Those that remain generally have a comment nearby explaining why they were not removed, usually due to some ambiguity or uncertainty as to whether or not it actually qualifies as a violation.

As mentioned earlier, while looking at the Issues page on the project site gives a general idea of what we worked on, all the members of Team Teams worked on just about everything in the project. Therefore, the Issues page does not give a clear picture of who was responsible for what. The number of issues per team member is fairly useless as a measure of determining if work distribution was equal; if I personally had many issues, it is because most of them were small and were sometimes typed up as I was completing the solution in question. (Of course, one might argue that working on small issues is an essential component of Issue Driven Project Management.)

The Jenkins page shows that most build failures were resolved fairly quickly. The exception is on 9 December 2011, when the system remained in a failed state for some time. This was detailed in Issue 36. Annoyingly, the solution to this had been known for some weeks, yet Team cycuc had not implemented the solution themselves.

A quick scan over the Updates page of the project site indicates that 28 of the 38 commits since Team Teams took over hale-aloha-cli-cycuc were associated with a particular issue. This means that about 74% of the updates were associated with an issue. This is an increase of a whole 3% from our work on hale-aloha-cli-teams, according to one of the technical reviews of our original project. Clearly, Team Teams is making great strides towards improvement in remembering to list the issues that they are making commits for. As mentioned above, updates were more sporadic and tended to be grouped together; nonetheless, work on the project was fairly consistent.



The most difficult part of working on hale-aloha-cli-cycuc was trying to understand a nearly foreign manner of doing things. In the technical review of hale-aloha-cli-cycuc, I stated that having implemented the project ourselves might have made it easier to understand what Team cycuc was doing. However, when actually working on the project, this almost had an inverse effect of that expected. Because we were so familiar with our project and our way of doing things, it was difficult to adjust to the new project. The system would be just similar enough for us to think that we knew how to create a solution, and then of course we would find out that the solution did not work at all, forcing us to learn about most of the code before we could accomplish anything. I do not believe that the continued development of hale-aloha-cli-cycuc under different management from the original owners would have been possible had my group members not been as dedicated and resourceful as they are, and I am honored to have contributed in a small way to their efforts.

In many ways, it is tempting to blame the difficulties that we had on the way that Team cycuc structured their system. It is difficult to tell where “different from how I would do it” becomes “who in their right mind would design something like this,” and admittedly one tends to think the latter more often when under pressure. We knew going into this that developing hale-aloha-cli cycuc would not be an easy task, having found several areas for improvement in our technical review, yet actually working on the project was much harder than I at least had anticipated. It certainly seems more productive to continue working on the same project rather than constantly moving to work on code that someone else has written; I am certain that we could have implemented the three commands within a week and a half had we remained with hale-aloha-cli-teams. However, this process of taking over projects from other developers, or even just cooperating with other developers, is undoubtedly what we will have to do as software engineers, and this exercise has certainly given us of Team Teams a lot of practice in that skill. It has also confirmed that all development teams should have a trained psychiatrist available, but given that I thought so before this project it is doubtful if that qualifies as a lesson learned from the experience.

Friday, December 2, 2011

Hale Aloha CLI Technical Review - Team cycuc

Some months ago, I wrote about eclipse-cs. To evaluate the plug-in, I referred to the Three Prime Directives, which serve as useful heuristics to examine the usefulness of a system. In theory, we as software engineers should always strive to keep these directives in mind when developing software. In practice of course, it is all too easy to become wrapped up in the excitement of programming and forget that we are creating this system for the users and future developers. Fortunately, I have a very skilled team of developers with a project that we have just finished work on, and perhaps even more fortunately there are two other teams of developers also working on a similar project. To ensure that we are all following the Three Prime Directives, we have conducted a technical review of the work that Team cycuc has done; in turn, Team cycuc has evaluated Team pichu, and Team pichu has reviewed the work of my group.

The Three Prime Directives are as follows:

1: The system successfully accomplishes a useful task.
2: An external user can successfully install and use the system.
3: An external developer can successfully understand and enhance the system.

At least intuitively, this seems to provide a good framework for evaluating the system. The Three Prime Directives, if followed, ensure that the system actually does something and account for everyone who could potentially have reason to interact with the system.

The system under evaluation is essentially the same as our own Hale Aloha CLI. hale-aloha-cli-cycuc is a command-line interface that allows the user to communicate with a WattDepot server, providing the user with data concerning energy and power usage on the campus of the University of Hawaii at Manoa. WattDepot is the cross-platform service that gathers, stores, and then provides the aforementioned data to the user.

Team cycuc has their code repository on a Google Code site, just as Team Teams does. Likewise, Team cycuc uses Jenkins for the purposes of continuous integration. Continuous integration, contrary to popular belief, has nothing to do with finding the integral of some equation infinitely. In this context, continuous integration is a tool that automatically verifies the current state of a project to ensure that the system is always in working condition; if there is an error then developers are notified immediately so that the problem, whatever it may be, is at least recognized quickly if not resolved promptly. Team cycuc appears to have been using Issue Driven Project Management, just as Team Teams did as described here.

Because of this similarity in projects and the use of identical tools and technologies, the process of reviewing hale-aloha-cli-cycuc is much easier for Team Teams to accomplish. The text of our review is as follows:

Review Question 1: Does the system accomplish a useful task?

Below is a sample run of team cycuc’s system:



When we initially ran Team cycuc's .jar file there was a slight problem as it could not successfully run. Eventually one of cycuc's team members had updated the system for it to successfully run in console. For the most part, this system provides functionality as described in the assignment specifications. For example, the formatting of both the date and power / energy is not in the same as the sample output given.



This may be an issue for some people depending on how they plan to process the data given. Reporting the data given in units such as kilowatt may be more desired. In the case of the rank-towers command, no units appear next to the output given which may confuse those not familiar with the system. Also, it appears that for the commands daily-energy and energy-since reports the wrong units with respect to the data given. From personal experience with the getData() method, the data returned by this method must be converted correctly to M Wh. In this case, 549 kWh should be 0.549 M Wh.

For example, the formatting under the current system is as follows;



Some of the commands do not successfully return data at all, below is a sample of the code when the rank-towers command was executed.



Essentially the system attempts to implement the four commands listed in its help menu. The exact usefulness of this system is debatable as we deem this version of cycuc’s system not ready for distribution.

Review Question 2: Can an external user successfully install and use the system?

In addition to containing the files for hale-aloha-cli-cycuc, the project site provides a very general idea of what the project is and does. The home page has a brief description of the system and a picture that presumably provides an explanation for the group name. This does give viewers an idea of what the system does, but not a very clear concept. There is no User Guide page; instead is a page titled “PageName” that contains most of the information that the User Guide should. The exception though is how to execute the system, which is not covered. The distribution file in the Downloads section does include a working version of the system along with an executable .jar file. The version number is included in the distribution folder name, allowing users and developers to distinguish between different versions. These version numbers include the timestamp corresponding to the time at which the distribution was created, thus letting users and developers compare versions chronologically. The numbers that actually indicate major and minor versions appear to have remained at 1.0 since the first downloads became available.

The tests of the system are shown below:

Valid Input:


Invalid Input:


Review Question 3: Can an external developer successfully understand and enhance the system?

The Developers’ Guide wiki page on the cycuc project site provides clear instructions on how to build the system in Ant. The guide also includes information on the automated quality assurance tools used on the project. Specific information about those tools is not given, but developers are informed that the verify task will run all of the automated quality assurance tools. A link to the formatting guidelines serves to document the stylistic rules that the code is to follow. The Developers’ Guide does not mention Issue Driven Project Management or Continuous Integration. Similarly, instructions on how to generate JavaDoc documentation are not available, though the documentation does appear to come with the project in /doc.

JavaDoc documentation, as mentioned above, comes with the project in /doc. However, developers may still generate JavaDoc files through Ant or Eclipse. The JavaDoc documentation itself tends to be well-written, though there are some questionable points and the description is somewhat sparse. Several methods lack descriptions in their JavaDoc documentation. There are a few contradictions within the documentation, as in CurrentPower.java where the description for the printResults method (line 28) indicates that the text printed is based on days[0] while the parameter tag for days (line 31) states that days is ignored. However, the JavaDoc documentation did show the organization of the system, and the names of the various components were well matched with their actual purposes. The system does appear to have been designed to implement information hiding, with the Command interface serving as an example.

The cycuc system builds without errors in most cases. A timeout while attempting to access the server will cause the entire build process to stop, which accounts for the instances in which the build fails. Aside from timeouts, the system builds properly.

The data that Jacoco provides concerning test coverage does induce some slight concerns about the validity of the testing. The halealohacli package has no testing at all. Testing on the halealohacli.processor package covers 67% of the code and 58% of the possible branches. For halealohacli.command, 94% of the code was executed in testing, while 59% of the branches were taken. (These values seem to vary upon repeated testing; this may be due to the aforementioned timeouts.) These low values for branch coverage in particular may stem from a lack of testing for invalid input. As a result, none of the exceptions are checked. The tests indicate that parts of the system work for a particular input; however, as there is only one test per test class (with the exception of TestProcessor) it is difficult to be certain that the system does behave correctly. Thus, the existing testing will not necessarily stop new developers from breaking the system; the testing ensures that developers cannot treat valid input incorrectly, but does nothing to stop invalid input from causing problems.

With regard to coding standards, there exist several minor deviations from the standards that do not affect the readability of the code. The amount of comments varies: at times, there is a comment explaining every line of code, while at other points there are entire blocks of code without any documentation. The deviations from the coding standards are provided below:



Overall though, the code is readable; admittedly, the person testing the code had already implemented the project for a separate group and thus might be familiar with the objectives of the code, which would affect the results and opinions of the tester.

Looking through the Issues page associated with this project, it is clear what parts of the system were worked on by each developer. This team utilized a variety of status options available to better inform an external developer what worked and what didn’t work with respect to project progression. In some cases, clarification in the form of comments show the decision making process this team used when dealing with issues. Since each issue described clearly explains what the task was, it should be easy for an external developer to determine which developer would be the best person to collaborate with. In terms of work input from all of the developers, it appears that some team members did more than others.

Turning to the CI server associated with this project, it appears all build failures were corrected promptly with a maximum latency of roughly 30 minutes. Also, looking through each successful build, this team showed that they were working on this project in a consistent fashion where at least 9 out of 10 commits associated with an appropriate Issue.