MyIsernReview

Code Author: MyISERN-1-Blue: Marcius Bagwan, Sonwright M. Gomez and John Hauge
Project Page: http://code.google.com/p/myisern-1-blue/

Installation:
Installation is smooth. All ant check pass. But when I import the project into Eclipse, there is a warning of unused import: import static org.junit.Assert.assertTrue; in TestMyIsernXmlLoader.java

Code format and Convention:
There are a few violations:

File	Line	Rule	Comment
MyIsernXmlLoader.java	119,152	EJS-9	Use meaningful name
MyIsernXmlLoader.java	44,45,46	EJS-39	Document all members
MyIsernXmlLoader.java	218	EJS-48	One sentence summary

Test Case:
As all 4 methods, which are new from example, return void, there is nothing much to assert in test.
In white box perspective, the test coverage is 100% according to EMMA, so it is well done.
In black box perspective, there are only a few conditional statements and all of them all satisfied at least once.
However, I found some problems:
1. Two format mistake when printing organizations. First, it prints the country as "CounterUSA", which should be "Countery: USA", I think. Second, name is printed like "Name:University of Hawaii", where I think a space is missing because every other places in the table, ":" always follow by a space.
2. Though he program has a good command-line arguments and also help message, the program exit without any word when using error argument. Help message should come out in this situation.
3. If the xml file contain an empty element like , or missing a element which would contain other elements, the program will end with NullPointerException. However, these kind of xml files will fail when running validateXml.build.xml. Therefore, the mistake of the program will be running without validate the XML files first.

Summary:
I am surprise that though code format and convention has been used and emphasized for so long, there are still violations in our codes. It seems to be easy to follow exactly the rules required. But actually, it isn't. Lots of practice is needed.
About test, though methods return void is evil and hard to test with assertion, we still ought to do it seriously, at least to guarantee that the output looks right, such as correct message with proper format.
Null object are evil and can usually break down a program. Most of the reasons when it happens in actual application is because of illegal input, for example, miss some essential parts or mis-structure format. So we really need to make sure to validate input before processing it.