Written on 10:46:00 AM by S. Potter
Everyone has been talking about Rails for a few of years now as the framework that makes it "stupid easy" to create web applications. The problem I have with this mentality is it creates a "stupid easy" sub-culture within the Rails community that promotes stupid laziness (as opposed to smart laziness). Over the last 3 Rails contracts, I have worked with supposedly experienced developers that have switched to Rails from OO languages like Java and C++ (as opposed to pretend OO languages like PHP). The results have been VERY disappointing. The biggest problem I see is that some, even quite experienced developers, have the [stupid] lazy mentality burned into their brains. They want to visit blogs and copy and paste code into their applications without thinking about it. In fact, recently I found a ridiculous real world example of such madness. No thought put into the pasted code AT ALL. I could identify the blog posting on a popular Rails blog that had the EXACT same code. The variables weren't EVEN renamed to be meaningful in the current application. Instead a finance web application had references to customers and invoices in the controller and view code when there were no such entities/models involved in the application. Not only is this a maintenance nightmare, it show how little thought went into the code just to get a small AJAX effect that (a) wasn't difficult to write from scratch (4 lines in total after refactoring from the 11 lines of code used) and (b) was probably not even that necessary from the user experience perspective. These are the developers I think ought to return to their no-thinking PHP or Java/JEE recipe books and leave the Ruby world alone. Of course, the authors of "recipe" books should also bare some of the blame for encouraging the no-thinking hackers out there to join the Ruby world in the first place. Is that really the type of mindset we wish to harbor in our community? Look at the PHP and Java universes today for a reference point of where we will end up very soon if we are not careful.
1. Stupid Easy SchemasOne of the deadly sins I saw on my travels through their code was extending the "stupid easy" mentality in the form of not defining schema properly. Sure they *used* Rails' migrations, but they didn't really create a usable schema based on the use stories and cross-story functional requirements. While you might not want to use database specific features like triggers, stored procedures, or even foreign keys (the latter assumes you are using Rails-based equivalents in place of FKs), you should still make your schema sensible. For example,
In this example we created a table called
# Migration code create_table :countries do |t| t.column :code, :string t.column :name, :string end # Model code class Country < ActiveRecord::Base end
countriesthat has two string fields:
name. In this case the code is the ISO country code that the application is going to use for almost all country lookups in the database. We knew that pretty much from the beginning because of the nature of our application. (Note: this example is a little contrived as I had to change things so as not to expose too much about the application this is from). Not only do we know that the
Countrymodel will almost always be looked up by its code (from the initial set of use(r) cases/stories we need to implement). The ISO code is also always 3 characters long. We also know that these ISO codes are unique for each country. All these things we knew before we needed to create the schema. This is the important information that is missing from the schema and the model is missing relevant validation. Before committing this new model and migration for
Countrythe developer writing the code (one of the developers I am thinking of loves to tout how XP/agile and great his code is), should have had the following code on initial checkin (there really is no excuse IMHO):
While on my soapbox about this point, another developer I respect said this might be premature optimization. In this specific case, I totally disagree, however, this is a good point to make in general. You don't want to do premature optimization either. This is something we all need to be careful about and to be aware. This is the point where you need to think for yourself. There is no cheat sheet on these types of thinking points. The [stupid] lazy Rails developers should go back to hacking PHP senselessly or following J2EE/JEE blueprint patterns if they find thinking for themselves to develop their own rules of thumb too much work. Remember [smart] lazy is the way to go.
# migration code create_table :countries do |t| t.column :code, :string, :limit => 3 t.column :name, :string # You may even decide to cap the length of the name field too, but this is more of a DBA style thing. end add_index :countries, :code, :unique => true # Model code class Country < ActiveRecord::Base validates_length_of :code, :is => 3 validates_uniqueness_of :code end
2. Stupid Easy ModelsAnother cardinal sin I often see in both client work I have inherited from others or open source projects is dumbing down models. Instead of creating member methods of models that related to *what* they are and do, some Rails coders (usually not very experienced) write this functionality within the controller layer, which leads to drastic controller layer bloat. Which is really ugly. Remember if your code is starting to look ugly, do something about it. The way to fix this is *almost* always to create relevant member methods on the corresponding models that this functionality works on. One easy example of this might be to add an
authenticatemethod to the
Usermodel instead of having extra logic in the
ApplicationController. Think CRC instead of procedural.
3. Stupid Easy Controller MethodsThis is perhaps the most obviously harmful (from a security perspective) sin. While traveling through the Supid Easy Rails universe of code, I almost always see unprotected controller methods that are used as filters or for utility purposes in some capacity. This is a sin that I myself must confess to, though I haven't done so since the first 3 months of my Rails development around 2005 Q4. The following is typically what I see around:
The problem with this is that we can do the following (unless using resource routes where default routes are taken out):
# in SparksController def check_permissions return false unless current_user.has_permission?(:MANAGE_USERS) true end
GET /sparks/check_permissions?some_var=some_valueAnd perhaps hack into the web application. In this case we may not be able to do anything too interesting, but there are other scenarios that this could create a very open hole in the web application. The way to fix this and keep your team's sanity from a maintenance perspective is to protect these filter or utility methods by scoping appropriate with
private, which is not difficult at all. However, if you are a true Rubyist you will probably opt to keep your controller utility methods in
Acts::Asmixins or similar. If you enjoyed this post Subscribe to our feed