views:

48

answers:

4

I have a bunch of named scopes and have a method within one of them that I would like to share between the other named scopes. I've sort of accomplished this by using define_method and a lambda. However, there is still some repeated code and I'm wondering is there a better approach?

Here's a simplified example of what I've got. Assume I have a table of projects and each project has many users.

Within the User model I have...

filter_by_name = lambda { |name| detect {|user| user.name == name} }

named_scope :active, :conditions => {:active => true} do
  define_method :filter_by_name, filter_by_name
end

named_scope :inactive, :conditions => {:active => false} do
  define_method :filter_by_name, filter_by_name
end

named_scope :have_logged_in, :conditions => {:logged_in => true} do
  define_method :filter_by_name, filter_by_name
end

Then I would use it like...

active_users = Project.find(1).users.active

some_users = active_users.filter_by_name ["Pete", "Alan"]
other_users = active_users.filter_by_name "Rob"

logged_in_users = Project.find(1).users.logged_in

more_users = logged_in_users.filter_by_name "John"
+1  A: 

I would probably use a bit of metaprogramming:

method_params = {
  :active => { :active => true },
  :inactive => { :active => false },
  :have_logged_in => { :logged_in => true }
}

method_params.keys.each do |method_name|
  send :named_scope method_name, :conditions => method_params[method_name] do
    define_method :filter_by_name, filter_by_name
  end
end

This way if you wanted to add more finders in the future, you could just add the method name and conditions to the methods_param hash.

mrjake2
Thanks for the input. I'll test it out and see what happens. Would I need to use `send :named_scope` though? Why not just called named_scope directly?
Beerlington
It's because you're inside the each loop - you're no longer at the class level at that point because you're inside a block of code.
mrjake2
+2  A: 

Named scopes can be chained, so you're making this harder on your self than you need to.

The following when defined in the user model will get you what you want:

class User < ActiveRecord::Base
  ...
  named_scope :filter_by_name, lambda { |name| 
     {:conditions => { :name => name}  }
  }

  named_scope :active, :conditions => {:active => true} 

  named_scope :inactive, :conditions => {:active => false} 

  named_scope :have_logged_in, :conditions => {:logged_in => true} 
end

Then the following snippets will work:

active_users = Project.find(1).users.active

some_users = active_users.filter_by_name( ["Pete", "Alan"]
other_users = active_users.filter_by_name "Rob"

logged_in_users = Project.find(1).users.have_logged_in

more_users = logged_in_users.filter_by_name "John"

I see that you're using detect, probably to avoid excess hits to the DB. But your examples don't use it properly. Detect only returns the first element in a list that the block returns true for. In the above example, some_users will only be a single record, the first user that is named either "Pete" or "Alan". If you're looking to get all users named "Pete" or "Alan" then you want select. And if you want select then you're better off using a named scope.

Named scopes when evaluated return a special object that contains the components necessary to build the SQL statement to generate the results, chaining other named scopes still doesn't execute the statement. Not until you try to access methods on the result set, such as calling each or map.

EmFi
I forgot that named scopes can be chained - definitely like your solution better.
mrjake2
Thank you for the suggestion. I was aware that this was possible, but I was trying to avoid redundant db queries. In my testing I found that chaining other named scopes on `active_users` would essentially build the entire query again and execute it, rather than just filtering out the object from memory. My example was very slimmed down, but the actual code has a few joined models and conditions so this sort of optimization seems important.
Beerlington
+2  A: 

Here's an entirely different solution that is probably more in spirit with what the question was asking for.

named_scope takes a block, which could be any Proc. So if you create a lambda/Proc which defines the filter_by_name method, you can pass it as the last argument to a named_scope.

filter_by_name = lambda { |name| detect {|user| user.name == name} }

add_filter_by_name = lambda { define_method :filter_by_name, filter_by_name }

named_scope(:active, :conditions => {:active => true}, &add_filter_by_name)

named_scope(:inactive, :conditions => {:active => false}, &add_filter_by_name)

named_scope(:have_logged_in, :conditions => {:logged_in => true}, &add_filter_by_name)

This will do what you're looking for. If you still think it's too repetitive, you can combine it with the techniques in mrjake2's solution to define many named scopes at once. Something like this:

method_params = {
  :active => { :active => true },
  :inactive => { :active => false },
  :have_logged_in => { :logged_in => true }
}

filter_by_name = lambda { |name| detect {|user| user.name == name} }

add_filter_by_name = lambda { define_method :filter_by_name, filter_by_name }

method_params.keys.each do |method_name|
  send(:named_scope method_name, :conditions => method_params[method_name], 
    &add_filter_by_name)
end
EmFi
Beerlington
A: 

You can also do this with a second named scope.

named_scope :active, :conditions => {:active => true}
named_scope :inactive, :conditions => {:active => false}
named_scope :have_logged_in, :conditions => {:logged_in => true} 
named_scope :filter_by_name, lambda {|name| :conditions => ["first_name = ? OR last_name = ?", name, name]}

Then you can do @project.users.active.filter_by_name('Francis').

If you really need to do this with Enumerable#detect, I would define the filter_by_name method in a module which can then extend the named scopes:

with_options(:extend => FilterUsersByName) do |fubn|
  fubn.named_scope :active, :conditions => {:active => true}
  fubn.named_scope :inactive, :conditions => {:active => false}
  fubn.named_scope :have_logged_in, :conditions => {:logged_in => true}
end

module FilterUsersByName
  def filter_by_name(name)
    detect {|user| user.name == name}
  end
end

This adds the filter_by_name method to the class returned by all three named scopes.

kjell_