Does Agile Web Development With Ruby on Rails (Third Edition) Create Bad Coding Habits?

Question

Does Agile Web Development With Ruby on Rails (Third Edition) teach best practices as well as Rails coding?

My concern is that, as I use this book, I'm developing bad Rails coding habits resulting from the rather basic nature of the examples used in the book. Case-in-point:

The Product Model:

class Product < ActiveRecord::Base
  def self.find_products_for_sale
  find(:all, :order => "title" )
end

The Store Controller

class StoreController < ApplicationController
  def index
    @products = Product.find_products_for_sale
  end
end

The Store Index View

<h1>Your Pragmatic Catalog</h1>
<% @products.each do |product| -%>
  <div class="entry">
  <%= image_tag(product.image_url) %>
  <h3><%=h product.title %></h3>
  <%= product.description %>
  <div class="price-line">
  <span class="price"><%= product.price %></span>
  </div>
  </div>
<% end %>

Is it a best practice to declare a 'helper' function of sorts just to pull in all the available products in the catalog? Shouldn't they have just done this?

@products = Products.find(:all, :order => "title");

I understand that they were probably just trying to demonstrate class-level methods, but they don't add any caveat in the code stating that this isn't really how you're supposed to do this.

Answer 1

I think in this particular you could have done that.

But sometimes when doing complex finds, its better to abstract into a custom find statement. Applying model specific business rules etc, should be done inside the model instead of the controller. So its not exactly a bad idea.

You can even use named_scopes to do the similar things. check out for this for more

http://railscasts.com/episodes/108-named-scope

Answer 2

I can't speak for the entirety of the book, but from your example, I don't find that to be a bad coding habit, but a good one.

It's not really a helper method, it's a specific query on your Product class and should be named/declared as such.

In the future, should you need to change the functionality of how find_products_for_sale works—I find this naming a little weird, I prefer for_sale—then you can do that in the model and not have to touch your controller at all. If you have multiple actions in your controller that are using find_products_for_sale then you've effectively saved yourself a bunch of typing/potential headaches.

There are now named_scopes (and as of Rails 3, just scope) for these types of things which is a nicer way of stating them.