Saturday, November 7, 2009

Code Review of WattDepot-CLI Branches

After spending the first half of last week working hard to finish our version 1.0 of the WattDepot command line interface, my software engineering class took a break of sorts from straight programming to conduct a code review for our various branches of the code. We were each assigned two other implementations to review, meaning that each group would receive feedback from four or five other people. I was assigned to review the Elua and Eha branches of the project.

To conduct this review, each of us used the template that was provided by my teacher. Essentially, I went through the code, first ensuring that it compiled and passed the automated quality assurance tools and doing basic testing. After that, I looked through the JavaDocs for the code, as well as the code itself, to determine if there were any additional problems.

My full reviews for the two branches can be found here:

I will be giving a few of the main points for each branch here, but if you are interested in my full comments, please refer to the PDF files linked to above.

Elua branch
Overall, I found few major problems with this branch. All of the commands work, although some fine-tuning is in order to ensure that there are no problems with incorrect commands being accepted. I would say that the biggest area for improvement is in the JavaDocs, some of which are missing @param or @return tags, and all of which would feel vauge to a new user of the system. Testing of the branch was thorough, although it would be a good idea to test one or two of the cases in which errors occur, since this is the main reason for lack of coverage by the tests provided.

Eha branch
This system worked well, but I feel that there are two major points that would benefit from additional work before future versions are released. First of all, the organization of the classes and methods could use some work. As detailed in my comments, the UserCommandInterface class should be broken up into several new classes, based on the type of command being called. Second, the test cases could use improvement, as they check the handling of correct vs incorrect input, and not the actual information that is output by the system.

No comments:

Post a Comment