16.MyIsernReview

Author of reviewed code: Shaoxuan Zhang

Introduction
This was a very interesting exercise for me, as the person who's code I reviewed was my group member. Therefore, the code I reviewed is, at least partially, my own. This essentially provided me with an opportunity to look at my own product and try to critique it.

Installation review
Although I already had this entire project on my computer, I tried to download it and run it as though I had never done so before. Since there was no .jar file contained in the download package, I ran it by importing it into eclipse.

I was able to easily download the file from the link provided on Shaoxuan Zhang's blog entry. It was a simple task to import the project into eclipse and run it. I had no problems installing and running this software. It passed the verify.build.xml test without and errors. At this point I have no problems with the project.


Code format and conventions review
This was an interesting task, as I had previously read and worked on the code I was trying to review. Going over this code again just focusing on code format gave me a different outlook on the code. Despite my best efforts, however, I was only able to find one problem. A bit more white space between methods would have made the code slightly easier to read. My table of convention violations is only

File	Lines	Violation	Comments
ExamplePrinter.java, TablePrinter.java, ...	12,10,...	EJS-7	White space needed between methods

Test case review
This was a particularly difficult area for me to critique, since it was my job to test the code. I tried to look at my own code with a critical eye and think of what was lacking in my own test cases.

Black box perspective
From a black box perspective, this was a difficult program to test. The requirements were just that the output be a series of tables containing the data from the XML files. There was no specification as to the format of the tables. Therefore, creating general test cases that considered the output of the entire system was nearly impossible.
Despite this, a test that parsed the output tables and compared them with expected values was missing.

White box perspective
This are was quite thoroughly tested. There were many tests for boundary conditions and illegal values. There were 8 separate tests for exceptions, as seen from the output of running JUNIT. Looking at the output from emma, there is only one section of code that was not tested, and that was the exception handing within of ExamplePrinter, the class that contained the main function. This section of code would only run if there was something wrong with any of the xml files used by main(). However, since the file names are hard coded, I see no way of testing this section of code. I could find no other missing tests from a white box perspective.

Break da buggah
It took me quite a long time to break this code, but I was able to find a problem. If the XML file is of an incorrect format, the program does not always behave correctly. For example, if an element is repeated twice with two different values, only the second will be displayed. For example, I edited collaborations.example.xml by adding an additional < Description> element, such as:

< Description>
A graduate student at the University of Hawaii spent the summer working in the University of Maryland research group.
</Description>
< Description>
This is a second description that does not belong here.
</Description>

This invalid XML format is not caught by the system and the output table contains

...--------------------------------------------------
...| Years | Description |
...--------------------------------------------------
...| 2005 | This is a second description that does |
...| 2006 | not belong here. |
...--------------------------------------------------

Summary and Lessons Learned
I got a fresh new perspective on my own project, and I think I did a fairly good job. I think Shaoxuan Zhang and I did a good job on the structure, design, implementation, and testing of the project. Perhaps the only thing I would change is to add some more complete black box tests.

I tried to critique my own code fairly, but I think it is impossible when you already know so much about it. It is much harder to critique and break your own code.