How to evaluate the quality of Rails code

code-qualitycode-reviewsruby-on-rails

In a code review, what do you look for to assess a developer's expertise? Given an opportunity to look at a developer's work on a real-world project, what tell-tale signs are a tip-off to carelessness or lack of experience? Conversely, where do you look in the code to find evidence of a developer's skill or knowledge of best practices?

For example, if I'm looking at a typical Rails app, I would be happy to see the developer is using RSpec (showing a commitment to using test-driven development and knowledge that RSpec is currently more popular than the default TestUnit). But in examining the specs for a Rails model, I see that the developer is testing associations, which might indicate a lack of real understanding of Rails testing requirements (since such tests are redundant given that they only test what's already implemented and tested in ActiveRecord).

More generally, I might look to see if developers are writing their own implementations versus using widely available gems or if they are cleaning up code versus leaving lots of commented-out "leftovers."

What helps you determine the skill of a Rails developer? What's your code quality checklist?

Best Answer

It is generally easy to determine the quality of code, and it has nothing to do with Rails or Ruby.

  1. All functions should be between less than 8 lines.
  2. No function should have more than two levels of indent.
  3. Names should be communicative and explicit.
    3a. Variables in short scopes should have short names. 3b. Variables in long scopes should have long names. 3c. Functions in long scopes should have short names. 3d. Functions in short scopes should have long names.
  4. The code should read like well written prose and have very few comments.
  5. As you read the code you should not have to look up the definitions of the variables and functions being called. They should be obvious.
  6. Function calls should have 3 or fewer arguments, with a preference for fewer.
  7. Unit test coverage should be close to 100%, and the programmer should know the coverage number.
  8. Unit tests should be short, easy to read, and easy to understand. You should be able to understand the program by reading the tests.
  9. The tests should execute very quickly. Long running tests are a symptom of carelessness.

Look for signs of carelessness like commented out code, meaningless comments, needlessly shortened names, long functions, long argument lists, etc.

Ask yourself whether or not the code looks like the programmer cared.

If you want some Rails specific stuff -- look to see if there are business rules in the active objects. This is a symptom of careless design.

Related Topic