Tuesday, November 10, 2009

Code Review for WattDepot Cli - Part 2

After reviewing two WattDepot Cli projects that implemented by different teams and their comments of our WattDepot Cli project, I learned some lessons.
First thing I learned is that error message is very important. When I wrote my code, I saw there are some many exceptions to catch if I need to retrieve data for a source. I thought it is kind of tedious to compose the error message one by one. So why don't I just send one same error message "connection error". However, when I read the comments, both team criticize on this irresponsible error message by saying the error message should be more specific. In both their projects, they handle all kinds of exceptions separately. I think they are right. Error message is very important for users. If it is not very informative, user would never know why the program is not working.
Second thing I learned is that declaring instance inside a loop is not efficient. Before, I like to create a temporary instance in the loop to hold some value. This is very inefficient because I keep creating new object in the system which is not neccessary. Instead, I can declare it outside the loop, and in the beginning of the loop initialize the value of that instance. In this way, the performance of the program can be improved.
Third thing I learned is that reviewing the project that created by other people is not a easy task. Even though for a small project like WattDepot Cli, it still takes me some time to understand other teams' structure, global instance, and test cases. Therefore, programmers should try their best to code in a readable way so that future reviewers can catch up quickly.
Overall, I think review other people's code is a not a bad thing to do. Although it might be boring, you can always learn something that you never know.

No comments:

Post a Comment