Take the 2-minute tour ×

I'm learning rails and trying to set up a product library where the products will be displayed based on three elements: location, category and expiry date (products can have multiple locations and categories but just one expiry date). Products will be shown as long as their expiry date hasn't passed and location and category selection will be via dropdown menus.

I started writing this question while having difficulty with incorporating the location and category selection criteria which i found a solution to but any help on what could be done better is greatly appreciated.

I've used has_many through connections to create the connections between the products, location and categories.

Here's the models:

class Product < ActiveRecord::Base  
  has_many :categorizations  
  has_many :categories, :through => :categorizations  
  has_many :localizations  
  has_many :locations, :through => :localizations  

class Categorization < ActiveRecord::Base
  belongs_to :product
  belongs_to :category

class Category < ActiveRecord::Base
  has_many :categorizations
  has_many :products, :through => :categorizations

class Localization < ActiveRecord::Base
  belongs_to :product
  belongs_to :location

class Location < ActiveRecord::Base
  has_many :localizations
  has_many :products, :through => :localizations

Here's my controller. Location & category ID's are passed as params and the expiry date of the products must be greater than the current time:

class LibraryController < ApplicationController
   def index
     @products = Product.find(:all, include => [ :locations, :categories ],
                              :conditions => ['expiry_date > ? AND locations.id = ? AND categories.id = ?',
                              Time.now, params[:location_id],params[:category_id]])

So by passing the location_id and category_id params in the URL I can list products by a combination of both.

Is there a better way of achieving what I'm trying to do?

share|improve this question
Define "better". Does your code currently work? Is it throwing errors? Is it slower than you expect? What's the actual problem you want to be improved (i.e. what sort of help are you looking for)? –  jefflunt Sep 17 '11 at 16:12
I'm just wondering if the overall approach is correct or if I'm going at it the wrong way. The code currently does what I want but when I say better I mean is there a more conventional approach? –  shane Sep 17 '11 at 16:28

3 Answers 3

up vote 0 down vote accepted

Ok. No, I don't think there is a "better" way in this case. There certainly are "different" ways of doing what you want, but on the face of it, what you're doing is fine, and it doesn't scream out "this code is terrible!" or anything.

Questions of advice/style are tough to answer here, because ultimately the answer to them is, "search the web for what other people are doing in your situation, and evaluate/make the decision yourself if your solution seems conventional/logical," or these kinds of questions are answered via study of relevant books on the topic.

It's nearly impossible to answer a qualitative question like this, because:

  1. There's several ways to solve every problem, many of which are neither "right" or "wrong"
  2. There's always edge cases where people break the "rules", in which case even unconventional solutions can absolutely be the best way to do something
  3. You're the developer, the one building the thing. To some extent you're expected to take a leadership role, and decide what's best

The reason I ask you to define "better" is primarily because of #1 - unless you give us a specific outcome you're trying to achieve, all you'll get are (a) answers that are full of opinion, and not directed toward a specific goal or (b) simply a different way of doing something which may or may not help you. Therefore, they aren't very useful in practical terms.

share|improve this answer
Thanks for the input and I take all your points. I'll try and phrase other questions with definitive goals rather than subjective queries. –  shane Sep 17 '11 at 16:54

This will also do what you want:

@products = Product.find_all_by_category_id_and_location_id(params[:category_id], params[:location_id])

You can also user Product.where which is supposedly better than find.

For more information, Google "dynamic finders".

share|improve this answer
I had tried using something like your suggestion but got an undefined method error for the 'find_all_by_category_id_and_location_id' –  shane Sep 17 '11 at 16:49
Sorry, I did not see that you were using :through on categories in your product model. –  Max Sep 18 '11 at 4:39

You could also improve upon your solution by using, "Product.where" (preferred over find in rails 3.1) and also turn them into named_scopes in Rails like and chain them as required.

scope :not_expired, where('expiry_date > ?', Time.now)
share|improve this answer
Thanks for that, that helped with my line of thinking. –  shane Sep 24 '11 at 20:40

Your Answer


