Thursday, November 15, 2007

MyIsern-1.3-review

Group viewed: MyIsern-Purple

Installation Review:
The installation is not as smooth as previous projects. First failure is from the String.isEmpty() which is not supported in Java 5. I will have to keep fighting with this problem untill Java 6 for Macintonsh released. Then when I importing it into Eclipse, the buildpath fails. However, it is not difficult to fix and get the isEmpty thing fixed for me too.
Then PMD and Checkstyle run successfully. But the JUnit test fail. The first problem for its failure is because their working directory's name is different from project distribution directory's, which included the "purple", and the project then is deployed with the name myisernweb-purple. Therefore, the test get a 404 when accessing the page. After fixing this, 4 tests still fail, HTTP 500 are reported. In tomcat there are 2 exceptions pop out when tests fail. These may be some real bugs, but I did not see into the code to find out what is happening.
The installation clear enough for me to get the software run.

Code Convention Review:
Some unused private variables and methods are reported in Eclipse, which might be historical remaining.
Some class share a same java doc, which is probably copy-pasted.
The most important violation is that the class Id implements a raw type Comparable, which is highly not recommended in Java 5. The right interface should be Comparable. and then the compareTo method will take a String as parameter rather than an Object.

Test Case Review:
First I notice that the EMMA's setting is not correct that takes jaxb class into coverage comptation.
The test cases are far from enough. As reported in EMMA, most methods in ActionBean are not tested, and only 66% methods in model are tested. I am not sure if the coverage is effected by the failures in JUnit test.
One bug I meet is when click save with name field empty, the system will end up to an exception.

User Interface Review:
The user interface is pretty good! Not only the functionality, but the appearance. In researchers page, there are picture in front of each person. When adding/editing new researcher, I can easily choose an existed organization from a drop down list, or create a new one. In collaboration editing page, years and organizations can be selected from multi-select field.
However, one important function is missed: delete objects. Objects cannot be remove once being added.
The affiliate researchers field is missed in organization editing window. Maybe authors thought that is should be determine by researchers' organization. But there are no view page available, so I cannot see an organization's affiliate researchers by any means.
Though using multi-select field to deal with multi-value field is really a good idea, it will become hard for user when selections become numerous. When using last assignment's data as input, there will be tens of organizations in collaboration's selecting window. However, I don't have much idea of how to do it good, and this approach is actually the best I have seen so far. But I think it is necessary to figure out some better way to do it.

Summary:
MyIsern-Purple's 1.3.1114 distribution is overall amazing, in consideration of such a short develop time for it. I do see many things useful for my project in it. I am more convinced that user interface is significantly important. This application's functionality is better than mine. But when looking at its nice pages, I do feel that my application is not comparable to this one.

No comments: