Friday, October 24, 2008

The impact of project reviews

Having recently finished our first iteration of our DueDates project, Daniel Tian and I set up a frozen copy of the project code and a new wiki page requesting reviews of our project. As classmates in our software engineering class were doing similar, a "review trading" system was implemented. Each person was given three other projects to review and each project would be reviewed by six or seven people in hopes of eliciting a broad spectrum of opinions and corrections.

Getting the most out of a review

In order that the different segments of the code and documentation were adequately reviewed, given that the busy schedules of the reviewers allowed for only a limited amount of time reviewing the project, we broke up the project into several pieces and assigned 2 or 3 of our reviewers to each piece. We knew our reviewers ahead of time, but on reflection, in a more widely known project a request for review would probably not know all of its reviewers identities. In that case I would break up the reviews perhaps by first letter of login name. A-H for one section, I-Q another, and R-Z for the last. This of course is not without weaknesses. What if no one is in the A-H range? What if the people in that range are assigned to review GUI code when they are much better versed with databases or documentation? That issue could be good or bad. These are all issues that should be kept in mind when requesting a review. In any case we broke up the project so that each person should at least attempt to run the project, but then concentrate their review on various pieces that were closely related. The assignments can be seen here.

Though we knew many of our errors before hand, and the review was merely a formality in some ways, our reviewers found several good improvements to be made. Erin Kim made several comments that suggest better ways to identify the correct tables when searching the UH Manoa Library. Creighton Okada and Robin Raqueno added how the use of arguments could be made more general in error reporting, and that some errors were slightly misleading. Daniel Arakaki left a good number of error reports regarding problems and shortcomings of the Javadocs. Anthony Du suggested that the CommandLineParser class possibly be refactored to only deal with the parameter parsing, and not so much the actions associated with the parameters. This is suggestion we definitely have been putting some thought into as well. None of them noted that our project had no unit testing at all. I did not see any comments from Scheller Sanchez however I'm not sure the issue in this case. (edit: I mistakenly said Tyler Wolff did not comment, but forgot that he had emailed me directly.) It could very well be I did not set up the review correctly through Google Projects, as I was a bit confused myself on several parts of the review process, as I'll mention shortly.

My reviews of others

I was charged with reviewing duedates projects silver, gold, and yellow. As is often the case with looking at someone else's code, I waffled between "thoroughly impressed" and "wtf is going on here." Though I had specific assignments to concentrate on, I found that I quickly wanted to review a lot of areas. As each one went along, I applied what I'd found in previous projects to the current one, having already seen common small errors it was quick to find and comment when I saw them again. I found myself often making comments that basically amounted to "you might consider doing it this way as that's how I did it in my project". I made many comments regarding code that I did not feel was as general and easily extended as I felt it should be to account for the growth of the project.

Difficulties with Google Project

While I find it amazing and wonderful that systems like this exist. It is so easy to set up, and of course free, but using the system does not always go as planned. In one case I "lost" some of my review comments somehow and began starting over, only to find them reappear later. I also began receiving emails from other projects regarding other people commenting on the project, apparently because I had started the review process, which again I found strange since I knew I wasn't the first review in that case. Daniel and I also found out as a result of this process, that our discussion group was not set up correctly and we were not receiving emails regarding comments on our own project. All of this does take a bit of practice and getting used to it seems!

Final thoughts

The review process is important for any project, whether it be software engineering or baking a cake. A project needs a variety of opinions and an organized way of collecting them. We're lucky to have tools like Google Project Hosting to help us with these tasks, even if not always as smoothly as wished. Hopefully future reviews, and I know there will be plenty, will be smoother and even more productive.

No comments: