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:

	% java -jar hale-aloha-cli-cycuc.jar
	Welcome to the Hale Aloha Client Utility
	(c) 2011 Cybernetic Cucumber
	Please enter a command or type "help" for instructions:
	> current-power Ilima-A
	Ilima-A's power as of 2011-12-01T21:51:48.917-10:00 was 6022.45 W.
	> current-power Ilima-Z
	Caught an exception! Here's the message:
	Invalid source: Ilima-Z
	java.lang.NumberFormatException: For input string: "Ilim"
	> current-power Ilima
	Ilima's power as of 2011-12-01T21:51:48.644-10:00 was 36946.46 W.
	> currentpower Ilima-A
	Caught an exception! Here's the message:
	null
	> daily-energy Mokihana 2011-11-05
	Connected successfully to: org.wattdepot.client.WattDepotClient@595e2ccd
	Caught an exception! Here's the message:
	400: Range extends beyond sensor data, startTime 2011-11-05T00:00:00.000-10:00, endTime 2011-11-06T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Mokihana/energy/?startTime=2011-11-05T00:00:00.000-10:00&endTime=2011-11-06T00:00:00.000-10:00&samplingInterval=15
	Sorry, due to technical difficulties, the data from WattDepot has been lost, the current range of datais from 2011-11-24 to the current date.
	> daily-energy Mokihana 2011-11-23
	Connected successfully to: org.wattdepot.client.WattDepotClient@595e2ccd
	Mokihana 599090.508905 MW consumed on 11/23/11
	> dailyenergy Mokihana 2011-11-24
	Caught an exception! Here's the message:
	null
	> daily-energy Alpha 2011-11-24
	Caught an exception! Here's the message:
	Invalid source: Alpha
	java.lang.NumberFormatException: For input string: "Alph"
	> energy-since Lehua-E 2011-11-01
	Connected successfully to: Lehua-E
	Caught an exception! Here's the message:
	400: Range extends beyond sensor data, startTime 2011-11-01T00:00:00.000-10:00, endTime 2011-12-01T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Lehua-E/energy/?startTime=2011-11-01T00:00:00.000-10:00&endTime=2011-12-01T00:00:00.000-10:00&samplingInterval=15
	Sorry, due to technical difficulties, the data from WattDepot has been lost, the current range of datais from 2011-11-24 to the current date.
	> energy-since Lehua-E 2011-11-23
	Connected successfully to: Lehua-E
	Source: Lehua-E 842680.2600147277 MW consumed since 11/23/11
	> energysince Lehua-E 2011-11-23
	Caught an exception! Here's the message:
	null
	> rank-towers 2011-11-01 2011-11-09
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-01T00:00:00.000-10:00 to 2011-11-09T00:00:00.000-10:00
	Caught an exception! Here's the message:
	400: Range extends beyond sensor data, startTime 2011-11-01T00:00:00.000-10:00, endTime 2011-11-09T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Ilima/energy/?startTime=2011-11-01T00:00:00-10:00&endTime=2011-11-09T00:00:00-10:00&samplingInterval=60
	Sorry, due to technical difficulties, the data from WattDepot has been lost, the current range of datais from 2011-11-24 to the current date.
	> rank-towers 2011-11-23 2011-12-01
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-23T00:00:00.000-10:00 to 2011-12-01T00:00:00.000-10:00
	Mokihana-06-telco 155344.46293599904
	Lokelani-08-telco 167058.68038782617
	Lokelani-12-telco 187371.9512349472
	Lehua-10-telco 192533.53641187027
	Mokihana-08-telco 220119.67542159185
	Mokihana-04-telco 240457.41220232192
	Lokelani-10-telco 254815.66310146335
	Lehua-04-telco 268630.9699227102
	Ilima-12-telco 274597.70970271714
	Mokihana-12-telco 292672.6133775022
	Lehua-08-telco 299165.17572917975
	Lokelani-04-telco 310087.3293257849
	Ilima-06-telco 349775.9117999347
	Lehua-12-telco 353006.35747479834
	Ilima-10-telco 368281.8752386514
	Lokelani-06-telco 370527.92657851847
	Ilima-04-telco 373673.2206887137
	Mokihana-10-telco 395845.07759343274
	Ilima-04-lounge 451310.8236008771
	Lehua-06-telco 478236.0429008212
	Lehua-12-lounge 489673.9025399294
	Ilima-08-telco 518525.8281234242
	Mokihana-04-lounge 523931.1545179207
	Ilima-10-lounge 541279.6641302761
	Ilima-12-lounge 584695.2196249235
	Lehua-04-lounge 586914.7915891781
	Lehua-08-lounge 602212.6174999066
	Lokelani-06-lounge 627994.9510547668
	Mokihana-08-lounge 636716.3599805776
	Ilima-08-lounge 640348.0950053576
	Ilima-06-lounge 644687.6149855237
	Mokihana-06-lounge 648958.0862912377
	Lehua-10-lounge 662660.5395207293
	Mokihana-10-lounge 666968.4345828574
	Lokelani-04-lounge 685572.3800609494
	Lehua-06-lounge 686538.5048981942
	Lokelani-12-lounge 693632.4448471135
	Lokelani-10-lounge 730484.4709622292
	Mokihana-A 764388.5667202426
	Mokihana-B 804302.5492272368
	Ilima-A 824984.0442895908
	Lehua-E 842680.2600147277
	Mokihana-12-lounge 849453.1114229541
	Lehua-D 855194.0759325996
	Lehua-A 855545.7615118884
	Mokihana-C 856836.0354021695
	Ilima-E 859292.9293276407
	Lokelani-E 881004.3960820606
	Lehua-C 901377.7932290863
	Ilima-D 909561.5393689275
	Lokelani-D 985300.1340636925
	Ilima-B 994463.5267854584
	Lokelani-A 995659.7093867343
	Lokelani-B 998522.8776332852
	Mokihana-D 1062813.5121762902
	Mokihana-E 1142125.7248004563
	Ilima-C 1158873.9231287818
	Lehua-B 1164774.5477990154
	Lokelani-08-lounge 1394742.517221462
	Lokelani-C 1561801.1976092882
	Lehua 4619572.438487317
	Mokihana 4630466.388326395
	Ilima 4747175.962900399
	Lokelani 5422288.314775061
	> help
	Here are the available commands for this system.
	current-power [tower | lounge]
	Returns the current power in kW for the associated tower or lounge.
	daily-energy [tower | lounge] [date]
	Returns the energy in kWh used by the tower or lounge for date specified (yyyy-mm-dd).
	energy-since [tower | lounge] [date]
	Returns the energy used since the date (yyyy-mm-dd) to now.
	rank-towers [start] [end]
	Returns a list in sorted order from least to most energy consumed between the [start] and [end] date (yyyy-mm-dd)
	quit
	Terminates execution
	Note: towers are: Mokihana, Ilima, Lehua, Lokelani
	Lounges are the tower names followed by a "-" followed by one of A, B, C, D, E. For example, Mokihana-A.
	> quit
	%

view raw gistfile1.txt hosted with ❤ by GitHub

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.

	> current-power Ilima-A
	Ilima-A's power as of 2011-11-07 13:48:56 was 2.3 kW.
	> daily-energy Mokihana 2011-11-05
	Mokihana's energy consumption for 2011-11-05 was: 89 kWh.
	> energy-since Lehua-E 2011-11-01
	Total energy consumption by Lehua-E from 2011-11-01 00:00:00 to 2011-11-09 12:34:45 is: 345.2 kWh
	> rank-towers 2011-11-01 2011-11-09
	For the interval 2011-11-01 to 2011-11-09, energy consumption by tower was:
	Mokihana 345 kWh
	Ilima 389 kWh
	Lehua 401 kWh
	Lokelani 423 kWh

view raw sampleoutput.txt hosted with ❤ by GitHub

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;

	> current-power Ilima-A
	Ilima-A's power as of 2011-12-01T22:56:50.337-10:00 was 6756.08 W.

view raw cycucOut0 hosted with ❤ by GitHub

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.

	> rank-towers 2011-11-01 2011-11-09
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-01T00:00:00.000-10:00 to 2011-11-09T00:00:00.000-10:00
	Caught an exception! Here's the message:
	400: Range extends beyond sensor data, startTime 2011-11-01T00:00:00.000-10:00, endTime 2011-11-09T00:00:00.000-10:00: Request: GET http://server.wattdepot.org:8190/wattdepot/sources/Ilima/energy/?startTime=2011-11-01T00:00:00-10:00&endTime=2011-11-09T00:00:00-10:00&samplingInterval=60
	Sorry, due to technical difficulties, the data from WattDepot has been lost, the current range of datais from 2011-11-24 to the current date.
	> rank-towers 2011-11-25
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-25T00:00:00.000-10:00 to null
	Caught an exception! Here's the message:
	null

view raw cycucOut1 hosted with ❤ by GitHub

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:

	> current-power Ilima-A
	Ilima-A's power as of 2011-12-01T23:11:50.751-10:00 was 6333.76 W.
	> daily-energy Mokihana 2011-11-25
	Connected successfully to: org.wattdepot.client.WattDepotClient@2be2befa
	Mokihana
	509734.840423 MW consumed on 11/25/11
	> energy-since Lehua-E 2011-11-25
	Connected successfully to: Lehua-E
	Source: Lehua-E
	639210.4079087041 MW consumed since 11/25/11
	> rank-towers 2011-11-25 2011-11-26
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-25T00:00:00.000-10:00 to
	2011-11-26T00:00:00.000-10:00
	Mokihana-06-telco 15385.932595232502
	Lokelani-08-telco 16653.69566474599
	Lokelani-12-telco 19451.44859161321
	Lehua-10-telco 19645.025624631904
	…

view raw cycucValid hosted with ❤ by GitHub

Invalid Input:

	> current-power Ilima-z
	Caught an exception! Here's the message:
	Invalid source: Ilima-z
	java.lang.NumberFormatException: For input string: "Ilim"
	> daily-energy Mokihana 2011-11-33
	Caught an exception! Here's the message:
	Invalid timestamp: Invalid day 33 days are 1 - 31
	> daily-energy Mokihana 2011-11-25
	Connected successfully to: org.wattdepot.client.WattDepotClient@7114460
	Mokihana
	509734.840423 MW consumed on 11/25/11
	> energy-since Lehua-L 2011-11-27
	Caught an exception! Here's the message:
	Invalid source: Lehua-L
	java.lang.NumberFormatException: For input string: "Lehu"
	> rank-towers 2011-1124 alsdjaflakdsf asdlkfjasd;flkajs fklajsdlfk sdf;lkafj k;djf ;akdjf;lkasfajs
	jdkl;asfj s;dlfkjas ;dlkfj asdl;kfj asfl;kasdj fasdl;kf jasl;kdfj as;lkfja sd;lfkajs ;lfkjas f;lkajsdf
	asd;lkfjas l;kdfjas ;dlfkjasd f;lkasjdf;lkas jf;laksjf as;ldkfjasl ;fkjasd;lfkjas
	Connected to server successfully.
	Rank Towers by Energy consumed, from 2011-11-25T00:00:00.000-10:00 to null
	Caught an exception! Here's the message:
	null
	//this tests overflows the buffer

view raw cycucInvalid hosted with ❤ by GitHub

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:

	EJS-07: Include white space.
	There is a lack of whitespace in the test methods of TestRankTowers, TestDailyEnergy, and TestCurrentPower.
	EJS-13: Capitalize only the first letter in acronyms.
	HaleAlohaClientUI class capitalizes "UI" instead of only capitalizing the first letter. Admittedly, "HaleAlohaClientUi" might have been confusing to read.
	EJS-29: Qualify field variables with "this" to distinguish them from local variables
	In HaleAlohaClientUI:
	HaleAlohaClientUI:
	prompt (line 66)
	isFinished:
	finished (line 27)
	promptForOperation:
	scanner (line 130)
	In Operation:
	getString:
	string (line 35)
	In CurrentPower:
	getPowerConsumed:
	powerConsumed (line 24)
	In DailyEnergy:
	printResults:
	energy (line 44)
	In TestProcessor:
	testGetSource:
	processor (lines 37, 47, 59, 60, 75, 80, 81)
	EJS-30: When a constructor or "set" method assigns a parameter to a field, give that parameter the same name as the field.
	In Processor:
	setSource (line 135)
	setTimestamp (line 81)
	Note though that in both of these cases the methods are not actually setting the field to the parameter value.
	EJS-31: Use uppercase letters for each word and separate each pair of words with an underscore when naming constants.
	In HaleAlohaClientUI:
	prompt (line 34)
	In Operation:
	quit (line 10)
	help (line 12)
	currentPower (line 14)
	dailyEnergy (line 16)
	energySince (line 18)
	rankTowers (line 20)
	EJS-33: Keep comments and code in sync.
	In HaleAlohaClientUI:
	"When we have the processor class implemented..." (lines 38-39)
	The Processor class is already implemented as of this writing.
	EJS-35: Use documentation comments to describe the programming interface.
	In HaleAlohaClientUI:
	JavaDoc comments were used repeatedly where single-line comments would have been preferable.
	EJS-53: Provide a summary description for each class, interface, field, and method.
	In HaleAlohaClientUI:
	isFinished (line 26)
	In DailyEnergy:
	getEnergy (line 48)
	In EnergySince:
	getEnergy (line 49)
	In RankTowers:
	rankTow (line 56)
	In Processor:
	getTimestamp (line 179)
	getBeginningTimestamp (line 187)
	getEndTimestamp (line 195)
	ICS-SE-Java-6: Format JavaDoc summary lines correctly.
	In TestDailyEnergy:
	test (line 22)
	The first "sentence" in the JavaDoc documentation is "1." This does not adequately describe the method.

view raw cycucStandardViolations hosted with ❤ by GitHub

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.