Showing posts with label Watt Depot Code review. Show all posts
Showing posts with label Watt Depot Code review. Show all posts

Wednesday, November 11, 2009

1-on-1 Code Reviews

The Elua Branch had one-on-one code reviews today. I found this mildly useful in the respect that both parties were able to elaborate a little more on the reports. However, I found the reports much more detailed, and therefore useful. I think code reviews should be about going over every line in the code after the reviewer has read and understood the program, perhaps more then the author.


The reviews in general from the reports were very helpful simply because when someone else looks at your code, they notice things you didn't. For the next project I am hoping to get more time with the reviewer and hope they read the code, instead of glance at it looking for specific conventions.

Sunday, November 8, 2009

Code Review

It was my task to review the code of two watt-depot branches. I found that the projects were not completed to specification and hard to review because of this. However, I'll attempt to give insight and advice on what needs to be done, in terms of error handling and design.

Ewalu - The commands are all implemented and working, good job. I think this code needs a little more time reorganizing and it will be a really good project. Exception handling for errors needs to be worked on and error messages need to be given. There are NumberFormatException's, ArrayIndexOutOfBoundsException's and some design issues to be worked on. Design issues involving remapping the structure of the project into logically divided packages and classes. Currently the source is mostly in one file. Test cases should also contain tests that check how the program responds to invalid input and exceptions.
Click for Report

Ehiku - The Ehiku branch needs a bit more time as well to implement and convert the rest of the commands into a more organized project consisting of classes and branches. It seems currently the transition is underway in the way the commands are parsed and dispatched. I'd like to see this design completed. There are some mishandled exceptions in the code as well, IllegalArgumentException, ResourceNotFoundException and the code leaves '[]' after a result is printed to the screen. There are no test cases either. But as I said, this branch can easily clean this up and just needs a little more time to do so.
Click for Report