8
\$\begingroup\$

This function in my Profile model receive a hash from a search form and should filter the table depending on those values. I guess there is something like a pattern for such a standard behavior, right now I ended up with this (unfortunately I had to reproduce the code here, so it might not work for some reasons, but you get the idea).

Just a note: we're storing in the model some strings containing multiple data, each of these fields should be a one-to-many relationship (db normality violation). I give an example: country_preferences is a string filled up by an html select :multiple => true. In the string I then find values like: ["Australia", "China", "United Kingdom"].

  def search opts # This default doesn't work the way I would like = {:user_type => :family}

    #TODO: default opts value in method sign if possible
    opts[:user_type] ||= :family

    # initial criteria
    fields= "user_type= ?"
    values= [opts[:user_type]]

    # each field filters the list in one of these three ways
    staight_fields = [ :country, :first_name, :last_name, :nationality ]
    like_fields = [:country_preferences, :keyword, :languages ]
    bool_fields = [:driver, :housework, :children ]

    # cicle the options and build the query    
    opts.each do |k, v|
      if straight_fields.include? k
        fields += " AND #{k} = ?"
        values += v
      elsif like_fields.include? k
        fields += " AND #{k} like %?%"
        values += v
      elsif bool_fields.include? k
        fields += " AND #{k} = ?"
        values += v == 'true'
      else
        logger.warn "opts #{k} with value #{v} ignored from search"
      end
    end

    # execute the query and return the result
    self.registered.actives.where([fields] + values)   
  end

I think I can use more scope but how can I combine them together? In general, how can I make this code much better?

The boss, when introducing the project, said one-to-many relationships for that kind of fields would be a pain in the back. What do you think about this pragmatic solution?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Does the code you posted in your edit actually work? It seems like you're missing profiles = at the beginning of every line after the first and a final profiles at the end of the method. \$\endgroup\$
    – sepp2k
    Commented Feb 1, 2011 at 18:40
  • \$\begingroup\$ Good point! - I've already corrected my code and I'm not going to refactor, but that behavior (getting rid of all the assignment) can be achieved with a dynamic delegator explained by Ryan Bates railscasts.com/episodes/212-refactoring-dynamic-delegator \$\endgroup\$
    – ecoologic
    Commented Feb 11, 2011 at 23:21

1 Answer 1

6
\$\begingroup\$

Have you looked into using a pre existing gem for this? I found one called has_scope that seems like it might do exactly what you're trying to refactor. You would need to add a scope for each of your filters and then add some has_scope calls in your controller.

Here's a few examples. I'm not as familiar with Rails 3 scopes as I am with 2, so the code likely won't work as is.

Model:

class Profile < ActiveRecord::Base
  scope :country, lambda {|country| where(:country => country) }
  scope :keyword, lambda {|keyword| where(["keyword LIKE :term", {:term => "%#{keyword}%"}]) }
  scope :driver, where{:driver => true)
end

Controller:

class ProfileController < ApplicationController
  has_scope :country
  has_scope :keyword
  has_scope :driver, :type => :boolean
end
\$\endgroup\$
3
  • \$\begingroup\$ this plugin looks easy tiny and tested, I would avoid to install an external plugin to substitute a single function used once, but when I have some time I'm going to dive in the code, there should be something to learn, thanks \$\endgroup\$
    – ecoologic
    Commented Feb 1, 2011 at 9:24
  • \$\begingroup\$ thanks a lot, that was it, but you don't need to declare scopes like country or driver, as you can see in the question edit you have by default scoped_by_driver(true) that works the same. \$\endgroup\$
    – ecoologic
    Commented Feb 2, 2011 at 9:17
  • \$\begingroup\$ I didn't even know about the scoped_by_* methods, great tip! \$\endgroup\$ Commented Feb 2, 2011 at 13:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.