16.MyIsernReview

As always the goal of reviewing peer's code is to improve the ability to read, write, and test Java systems. Today, I will review Kevin's code who is a graduate student taking Software Engineering 613 at the University of Hawaii.

Initial Problem
I was assigned to review Kevin's code. When visiting his Blog entry for MyISERN-1.0, I did not find and links pointing to the distribution file. Technically, I would be done now, since I was not able to download any project to review. However, I like reviewing someone's code because most of times I learn more about the Java programming language. Therefore, I went through the list of names trying to figure out if his project partner had a pointer to the project distribution. I was glad that he had, because it saved my contacting Kevin by email and it also saved him some stress editing his Blog entry.

Installation Review
I have downloaded Kevin's project from this link. The download went smooth and I was able to unzip the distribution into my local directory. I also did not have any complication when importing the unzipped distribution into Eclipse. Right after importing the project into Eclipse, I decided to open the command prompt and try executing his code using this command: java -jar .... however, as it seemed there was no jar file present. Therefore, I tried to create a jar file using the ant command ant jar. The build was successful and a jar file was established. Now I was able to invoke java -jar myisern.jar and it printed the desired three tables within the prompt. Although, the tables could be formatted nicer, they seemed to have the correct values. (Tables look better in Eclipse) Running the command ant -f verify.build.xml did not return any errors and Emma coverage was at 100%. However, a 100% Emma coverage doesn't indicate that the system was properly tested which I will find out in the "Break da buggah" section of this assignment.

Code format and conventions
This section was pretty well covered. I could only find a few violations which are presented in the table below.

FILE	LINES	VIOLATION	COMMENTS
TestMyIsernTable.java	6	ICS-SE-Java-2	Do not use wild card "*" in import statements
TestMyIsernTable.java	1	EJS 41	Provide a summary discription (Empty)
MyIsernTable.java	82	EJS 32	Write documentation for those who must use your code and those who must maintain it
MyIsernXmlLoader.java	123	EJS 1	Adhere to the style of the original (no space)
TestMyIsernXmlLoader.java	18	EJS 1	Adhere to the style of the original (@Test)

Test case review

Black box perspective

There are tests that test if the correct values are read from the XML files. The method testGetResearchersTable() tests at least one field from each researcher. However the field picture-link and Bio-statement are never tested. Therefore, I would suggest to test these two values at least in one researcher. In the second test method testGetOrganizationsTable(), I would suggest to also write a test for Affiliated-Researchers. Since for one researcher this value is null and for the other not. Checking this test case, made me wonder if the table output is correct. That is why I ran the program. I figured out that it doesn't even print the Affiliated-Researchers. Then I found out that for other tests it is similar. Wherever I thought there is a test case missing, it did not print the values to standard output. Now, I am thinking if this was done with purpose, because otherwise the content couldn't fit onto the screen the way the table was formatted, or were the values left off unintentionally.

White box perspective

The testing is well done from the white box perspective. I ran Emma on this program and it returned a 100% coverage for all areas. Job well done! But, I suggest not to rely on that result. After knowing that the code is covered 100%, I would advise to close the coverage report and look at the test cases from a block box perspective again. Make some logical decisions whether to test each important part or just do testing for certain values.

Break da buggah

When running this program it does not return all the values contained in the example XML files. Does this mean that it returns incorrect values? Other then that, even if I run it with additional arguments it still prints the tables. On the one had it is good because this doesn't mean that the program crashed, but on the other hand it would be useful to the user if a statement would be printed informing that additional arguments are not supported at this time.

Conclusion

I believe that for this assignment we supposed to print ALL the values contained in the example XML files. However after reviewing Kevin's code, it made me wonder, because that code did not print all of the values, at least I didn't see them on the console. Especially, because the program seemed to work and look right. Certain values were missing in the output. Now, imagine if this would be distributed to the customers and they could not find the needed values in the just so expensive bought software. Seems like I learned that designing logical test cases and checking the output is very crucial to the correctness of a software output.

15.MyISERN-1.0

Project Links
Distribution package
Project Hosting
Discussion Group
Svn Group

Using JAXB
Once we figured out how to use JAXB, this assignment seem to be easy. However, learning a new tool isn't always as easy as it is described on several websites. I learned a lot by trying to figure out how to use JAXB to extract values from a XML file. As mentioned it was not that easy at first. I went to some "help" websites that had example code of unmarshalling and marshalling. I took the exact same code from various websites and changed some variable names. At first it didn't work. But after thinking and rewriting that code, it suddenly made sense how JAXB works and it was fairly easy to continue with the assignment.

Implementing Tables
After we accomplished this task, we debated on the format of the tables. Since in the assignment instruction it was never clear if the tables should be command line implemented, we decided to take it a step further and create nice tables which use the Java swing package. However, after reading assignment 16 (Next Assignment), it seem like we should rather stay with System.out because it would be easier for implementing assignment 16.

What about XML
Even though we were working with XML files, a person not knowing XML would also be able to complete this assignment. This is because the XML files, as well as the XML schema files, were provide by the professor which saved us a lot of detailed work. This is also why we were able to fully concentrate on our actual task; learning to use JAXB. In addition, JAXB did all the work for us. Nevertheless, it was still a hard assignment because this was the first assignment which required working in groups and it also required using TortoiseSVN which none of our group members used before. Therefore, we had to learn to use this tool as well, within a short time period.

Group development
Actually, I must say that group projects as experienced this time make really fun. Not only because I didn't have to think about the code implementations all on my own, but also because I learned a lot of new "tricks" in using Eclipse. I was also able to see other peoples' way of coding and learned a different process of thinking about implementing certain methods. I was actually, surprised that we were able to meet every day for at least one hour. I was surprised because each of us work on a different shift and each of us still has other homeworks to do. That was amazing one of the member works mornings, I work afternoons, and another group member works nights. But we still figured a time to meet and there was never a situation where one member had to wait for another member to appear to the group meeting. So, this was a really good experience.