The 4 Things We Look For in a Code Review
Most developers think that every “other” developer’s code is “no good.” In fact, it is exactly this “Not Invented Here” syndrome that makes it dangerous for other developers to evaluate an existing project’s quality without a checklist or template as a guide.
Here are a few of the things in Highgroove’s Code Review tool-belt, that we look for:
Tests – This is a given – Ruby on Rails ships with a great test framework. Your app needs tests. The default suite will do! Many developer teams will even upgrade the testing suite, adding RSpec and Cucumber, Selenium for in-browser testing, rcov for code coverage, and FactoryGirl for fixtures and test data. Without tests, you’re bound to regress (“Jim fixed a bug, but looks like he added a few more…”). Bottom line: Tests = Quality.
Idiomatic Ruby and Rails – Did the developers use standard plugins, or did they re-invent pagination? Did they use a search plugin or did they write their own fancy-pancy algorithm for search? Are they using helpers, or just copy-pasting code all over the place? This can tell a lot about a code-base and how easy to maintain it is and will be long-term.
Code Look and Feel – We are actually looking for indentation, comments, and commented-out code here. This may seem strange, but projects that have lots of sloppy indentation means a developer didn’t have much respect for the code long-term. If they’ve got a lot of code commented out, it appears they “got something working” and then didn’t bother cleaning up. It’s the equivalent of a mechanic who fixes a car, but leaves bolts lying around, loose, dings up other pieces, and doesn’t clean up. Yikes!
Bootstrapping Documentation – How easy is it to get a new Developer up and running? This may seem strange to check for, but just looking to see how well the current Developers expect other Developers to be able to get up to speed is a sure sign of a quality application. It takes a high level of maturity to realize, as a Developer, that you will not always be the only one working on a project!
Even when we come across a project that may not contain many of these quality indicators, it’s not the end of the world. Once we’ve identified the broken windows, we can get to work fixing them.