Wednesday, March 5, 2008

Ambient Team Code Review

The Ambient Device team gave a presentation on their current release for us to do a review of their code.

By following the installation and developer guide, I was able to check out and install the project. Installation was not difficult as I already have Hackystat installed and have a valid Hackystat account, so I don't have to go through setting up Hackystat again. Everything was fine until I try to run verify. According to the developer guide, it mentions that junit test should fail but the problem is it didn't mention which test should junit fail. So I am not sure if I am just having the error that the ambient team have or if I actually have additional problems that have to be fixed. In the end, I came across three major errors and spent a lot of time just to fix the first two. However, there is still an error: org.apache.xerces.dom.DocumentImpl.getXmlStandalone()Z that I didn't know how to be fix.

For the code itself, it was pretty organized and I don't see much problems with it but in term of documentation, it could have been better. Problems or areas that can be improved:

1. Even though the code is pretty organized, there is a lack of comments. There are comments for each class and methods but comments within methods are rare, which makes it harder for the user to understand the code.

2. The superclasses could have been more comprehensive instead of having only one method.

3. When the jar file is invoked, there is only a statement saying "LastBuild: Active" which is not helpful as it doesn't give the user any information whether it is actually communicating with the orb successfully or not.

4. For documentation, a troubleshooting section would be really helpful. Apparently, many of us are having the same junit errors that makes verify fail such as the Jaxb api problem and the javamail installation problem. So if they can provide solutions to these problems, it would make the developer guide more comprehensive and helps the user.

5. There should be instructions or references instructions to install Hackystat in their developer guide. It would be confusing for users who have no prior experience with Hackystat.

6. Another obvious problem is it takes too long for the orb to change color. This is a problem because during the wait for the orb to change color, the project situation could have been changed completely. For example, it could have gone from a failed build to a successful build again.

It was a fun experience to do this review as it gave me to chance to play with the ambient orb for the first time. The ambient team has surely got a good start with their project. There is big potential to this project. As mentioned above, I think their code is working well and the main problem is with the documentation and the color changing latency, both of which should not hard to fix as the latter could be fixed by acquiring the appropriate hardware. I look forwards to see what the ambient team is going to do with the Nabaztag as well.

No comments: