Rails App Suck Factors
I’ve been working with Rails since very close to its beginning, and over the past few years I’ve been asked to review a number of Rails apps.
When you ask one programmer/developer/engineer/whatever about another person’s work, they’re invariably going to tell you it sucks. The question then is: how much does it suck?
Here are some general things to look for:
- Lameness is directly proportional to the number of plugins. This is almost always the case. If the people working on your Rails app are hauling in plugins they don’t understand, chances are your app is going to suck and be filled with cut-n-paste code. The first thing I do when I review an app is look at the gem dependencies and the contents of vendor/plugins.
- Tests that don’t pass are a huge warning sign. Rails people love to talk about testing, but in my experience not many do it. If I can’t get the tests to run, chances are there have been multiple developers on the project and the one who did care enough to write tests has moved on.
- Lack of tests / dumb tests. I’m not going to tell someone they have to have 100% test coverage. But if every test case in the app just has a test_truth method in it then your developers are lazy. Rails couldn’t make it easier to write and run tests; there’s no excuse not to write some tests.
- Rails 1.0-style controllers. If I see controllers that have methods like “list”, “add”, etc., this is another tipoff that the developer doesn’t care to keep up. Using restful controllers gives you enough for free that it’s foolish not to do it.
- The routes file. If there’s only the default route, then you’re going to have the Rails 1.0 controllers problem. If the routing file is a hodgepodge of vanilla routes, named routes, and restful routes, the project is probably either really old or really messed up. There’s virtually no reason to create an anonymous route.
- Super-sized controllers. This has already been beaten to death, but it’s also a huge tipoff that the app sucks.
- TODO comments everywhere. When I’m brought in to review an app, the people working on it usually know. So, they’ll put comments like “TODO - make this better” all over the app. This doesn’t make me think “wow, these guys are really on the ball”. If the fix is about the same effort as writing the comment, then just make the fix.
- Lots of raw SQL. If developers are using sql to “work around” ActiveRecord, chances are they don’t know how to use it.
- Click and count queries. Click through the app and check the logs. I’ve seen a relatively simple app that had a main page that generated over 1,000 queries. The results of most of those queries were discarded (a really bad before_filter was the culprit). Again, lazy.
- Working against the conventions. This is implied by some of the above, but is worth calling out. For instance, if someone doesn’t understand how autoloading works and has explicit requires in the app because they couldn’t be bothered to name a file correctly…yep, that’s just lazy. Incidentally, a peek in the /lib directory is usually an early indicator of how good or bad the app will be.
There are plenty of others, but in my experience these are the big suck factors.