Sunday, May 10, 2009

Code Reviews

My current job sometimes entails reviewing other Java programmers' work and providing solutions for "better" ways to implement something. Sometimes I find outright bugs, but usually my recommendations are for more efficient approaches.

How do we determine one approach is better than another objectively? Profile!

Run the application's functional and/or unit tests through the code path in question with a profiler. Check for object allocations, time spent in methods, lock contention, and the like. Decent profiling tools provide this functionality, but the best ones I've used provide better visibility into these areas and aren't free.

If your approach can't be shown to exceed the original objectively, don't hesitate to throw it away. On the other hand, don't throw away an approach you believe should be better because the first implementation doesn't blow the doors off the original.

When afforded the time, spend some time looking through profiler results searching for the obvious inefficiencies. Balance the desire to wring every last byte, microsecond, bytecode instruction, etc. out of an approach with source code maintainability and the value of your time on other tasks. Spending a week working exclusively on a 5% improvement is the exception, not the rule.

Objectivity has one added benefit during code reviews as well, particularly for "outsiders" looking at a team's code. Objectivity helps defuse the nasty politics and ego bruising that sometimes (usually?) accompany code reviews. Code reviews have always been a learning experience for me and should be for everyone. Taking pride in one's work is one thing, but taking a review of one's work personally is quite another. How many times have we reviewed our own code days, months, years later only to say "What was I thinking?" A review from a colleague should be no different and offers a chance to fix mistakes before they become costly issues.

No comments: