Challenge: given the following class, write a method #find_like_objects which will call the class method .find with attributes.

class Foo
  attr_reader :attributes

  def initialize
    @attributes = { :bar => 42 }
  end

  def self.find(attributes)
    "Foo.find"
  end
end

Simple enough:

class Foo
  # ...
  def find_like_objects
    Foo.find(attributes)
  end
end

o = Foo.new
o.find_like_objects             # => "Foo.find"

OK, let’s add a requirement. The method #find_like_objects method must be in a module, which can be included in any class that defines .find.

That’s not a problem, we just need to make the method a bit more generic:

module Finders
  def find_like_objects
    self.class.find(attributes)
  end
end

class Foo
  include Finders

  # ...
end

o = Foo.new
o.find_like_objects             # => "Foo.find"

We should check that it still works even if .find is defined in a superclass:

class Parent
  def self.find(attributes)
    "Parent.find"
  end
end

class Foo < Parent
  include Finders
  # ...
end

o = Foo.new
o.find_like_objects             # => "Parent.find"

Looks like all’s well. Are we done?

Well, if we want #find_like_objects to be truly generic, it should also work when .find is defined in a module. Does that work with the implementation above?

module Utils
  def self.find(attributes)
    "Utils.find"
  end
end

class Foo
  include Utils
  include Finders

  # ...
end

o = Foo.new
o.find_like_objects             # =>
# ~> -:70:in `find_like_objects': undefined method `find' for Foo4:Class (NoMethodError)
# ~>    from -:92

I guess not. What can we do to make it work? We could iterate through the list of ancestors to find the first one that responds to .find:

module Finders
  def find_like_objects
    self.class.ancestors.detect{|c|
      c.respond_to?(:find)
    }.find(attributes)
  end
end

class Foo
  include Utils
  include Finders

  # ...
end

o = Foo.new
o.find_like_objects             # => "Utils.find"

That’s it, right? Now we have a fully polymorphic #find_like_objects method that will work no matter where .find was defined, right? Well, yes, unless of course the .find method is defined in a singleton class…

o = Foo.new
class < < o
  def self.find(attributes)
    "Singleton .find"
  end
end
o.find_like_objects             # => "Utils.find"

Our nice generic #find_like_objects method ignored the singleton class and used the module version instead. What to do?

module Finders
  def find_like_objects
    class_chain.detect{|c|
      c.respond_to?(:find)
    }.find(attributes)
  end

  def class_chain
    [(class < < self; self; end), *self.class.ancestors]
  end
end

class Foo
  include Utils
  include Finders
  # ...
end

o = Foo.new
class << o
  def self.find(attributes)
    "Singleton .find"
  end
end
o.find_like_objects             # => "Singleton .find"

At last, we have an implementation of #.find_like_objects which will call the nearest-defined .find no matter where it lives. But it took eight lines of code to do it! The astute reader has probably already realized what we’ve done here: we’ve reimplemented Ruby’s polymorphic method searching rules manually. In a word, BLEAAARGH!

The lesson here? Don’t use class methods when you need polymorphic behavior. I would generalize that to say prefer instance methods to class methods where possible. In some languages, the rule is to use a class-level (or “static”) method for any method that doesn’t need access to instance variables, but in Ruby it’s usually better to define an instance method unless you absolutely need to call the method from the class level. You run into similar issues with class-level instance variables.

Published by Avdi Grimm

13 Comments

  1. I think the real problem in the Utils module is that the module method never gets added to the class to begin with. Even if you call Foo.find directly you will still get method missing. What you really want for Utils is:

    module Utils
    def find
    #…
    end
    extend self
    end

    And then extend Utils in Foo instead of including it. That way the method actually gets added as a class method of Foo, and you maintain the ability to call Utils.find. Then the only complicated checking you have to do is to check to see if the object responds to find before calling the class method.

    Reply
    • It's a fair point, but it serves to emphasize the fact that class methods just don't parallel the instance method search chain. If you actually want to use Utils as an includable module (e.g. it also has instance methods you want) then you have to do something like Paul suggests elsewhere in the comments, a verbose pattern I personally dislike.

      Reply
      • IMO, people who understand singleton methods should not define them on modules they plan to use as mixins.

        What I do for this kind of thing (also ugly, but slightly less so): http://gist.github.com/579194

        I don't think class methods are to be considered harmful. But they ARE definitely abused.

        Reply
        • IMO, people who understand singleton methods should not define them on modules they plan to use as mixins.

          Precisely my point 🙂

          And you'll note I never used the word “harmful”.

          Reply
      • For the specific case of finders, I like Candy's approach: http://github.com/SFEley/candy

        I haven't used it, though, so I'm not sure how annoying it gets, but TL;DR: You define an extra class for each of your models that handles the collection. So for the Person model you would also define People (which includes Candy::Collection). Person is blissfully unaware of finders and that sort of thing. People handles all that.

        It feels much cleaner, even if a bit more verbose. Also, I keep typing People.all in the rails console because my mind tells me that I want a collection, so it probably maps better to my brain 😛

        Reply
    • Incidentally, this is from some real code. There are some metaprogramming cases where the included instance methods need access to a class-level method that was in the module they were included from – but the actual class that the module was included in doesn't need/want those class methods itself. And if the module was generated rather than named (as in these examples) it's not possible to reference it by name.

      Reply
  2. I agree with Mike, and if requiring the users of this library to extend the module is painful, you can get around that with this:

    http://gist.github.com/579041

    Reply
  3. I don't know if the point is being driven strongly enough: class method of modules are NOT SUPPOSED to be sent down to classes that include them. Furthermore, your specification is pathological. You want a method that calls find on the parent class, and, if that fails, to go off and search a bunch of modules, calling find on THEM (if find is defined)? You then complain that there is no “clean” way to make these module methods available to objects of the victim class without adding them to the class itself?

    If you must…

    module FindMe
    module ClassClassMethods
    def find_me_module
    @find_me_module
    end
    end
    module ClassMethods
    def included(klass)
    klass.instance_variable_set :@find_me_module, self
    klass.extend FindMe::ClassClassMethods
    end
    end
    def self.included(mod)
    mod.extend ClassMethods
    end
    def find_like_objects(attrs)
    self.class.find(attrs)
    rescue NoMethodError => e
    if e.message =~ /^undefined method `find' for /
    self.class.find_me_module.find(attrs)
    end
    end

    An ugly solution to an ugly problem. But it avoids the problem of some innocent module that happens to have find defined on it from being picked up because someone is messing around with the call chain.

    Reply

Leave a Reply

Your email address will not be published. Required fields are marked *