Tuesday, May 25, 2010

The new fashion: Code Review

Is code review, no code ownership (5 people working on the same small feature), scrum, and monthly/weekly deliveries better than the traditional coding ?

Frequent deliveries rush the project at the beginning, skipping the design phase, thus the program will have a lot of almost duplicated code.

If there is no code ownership, when a new method is needed, it is just added to the existing class, because reviewing a patch that has added functionality plus refactoring is hard. And nobody wants to break already tested, existing code, close to a frequent delivery.

Maybe the waterfall model is not that bad after all. If I'm the owner of all the code of a small feature, and it has not been thoroughly tested yet, I'm a lot more willing to refactor and design the code properly. Otherwise there is a risk of stepping on each others' toes.

The errors caught on code reviews often mask a bad spec or a rushed up initial technical design. Trivial errors are often caught anyway by the author of the code or by the testers, 2 days after the code is written, and can be easily fixed. If people work on different areas of the app, I don't care that my collegue's area does not work as expected for 2 days. If it does not crash the program on startup, and does not crash my feature, I'm ok with it.

If 5 people work on the same small feature and everyone is working on each layer of the app, a lot of time is spent by each of them to learn the existing code. Doing something the first time can take 10 times longer than doing it the 5th time. So, no code ownership can waste 1 week of each of these 5 people's time just to learn the existing code, the "model" of how to add the new code. All these 5 weeks just to don't need to have 1 developer spend a week on ramping up on a collegue's code, when he eventually leaves the company. It seems a high price to me.

Code review is still useful to learn programming tricks from the collegues. But this learning review can be done more rarely.

No comments:

Post a Comment