Wednesday, August 31, 2011

Code Reviews: What I Look For

I'm currently working with a number of contractors, and all of us are working in the same code base. We each work on a separate feature branch, and when it's done, we merge to master and deploy to staging, and then to production.

One of the benefits of this is that it's easy for us to review each other's code. We just grab the feature branch and start looking. Usually we're developers reviewing each other's code, but sometimes we'll get a tester to take a look, or even one of our fearless marketing types (she's usually looking at language and naming).

Now, a lot of books and checklists will tell you useful and good things to look for:
  • relatively short methods
  • good encapsulation
  • conformance to style guidelines (get a tool to do this, if possible)
  • all the tests pass
  • error checking and input handling
  • readability and understandability
  • locations and placement (use of libraries, MVC conformance)
  • etc.
These are all great things. But wait.... there's more. We do code reviews because they sometimes offer us other benefits, like:
  • an example of an elegant way to solve a problem (if there's an elegant solution in that particular code snippet - and it's not uncommon!)
  • a basic understanding of how the new feature fits in to existing features. This is useful when we go to do another new feature later on - I'll probably remember that I should go look at this feature because it might be affected.
  • an interesting library (or gem or whatever your language calls it) that's worth looking at for other things
  • common code I might not otherwise have noticed was there for me to use
  • a chance for the developer who wrote the code to ask questions or point out worrisome areas
Bob Martin's Clean Code has a lot more information, and I highly recommend it. Then go forth and actually look at your code base. There's a lot of neat stuff in there!

No comments:

Post a Comment