Search code examples
ruby-on-railsrubyruby-on-rails-3modellaw-of-demeter

Ruby / Rails: create a class method that operates on instances of its children?


In my app, Photo has_and_belong_to_many :land_uses

I have this helper method in the Photo model:

def land_use_list
  land_uses.map(&:name).join(', ')
end

This strikes me as a code smell (demeter), but I haven't been able to figure out how to move it to the LandUse model. What I'd like to do is something like:

class LandUse < ActiveRecord::Base
  ...
  def self.list
    self.map(&:name).join(', ')
  end
  ...
end

So that instead of calling photo.land_use_list I could call photo.land_uses.list.

But that doesn't work, because it gets called against the class instead of being called against the scoped instances belonging to a particular photo.

Is there a way to do what I'm thinking of? And, more generally, how do you approach issues like this in your app? Is moving the list code to the LandUse model the right approach, or would you recommend something different?


Solution

  • First, I don't think this is violating the Law of Demeter per se. You have a method on an object that calls one method on an attribute to create a temporary variable, and then act on the temporary variable.

    It would be a violation of the Law of Demeter, if you were doing this from a different class entirely. eg

    class User
      def names_of_lands_ive_known
        photos.map(:land_uses).map(:name).join ', '
      end
    end
    

    As it is, it's just good information hiding. But, if you wanted to be able to write photo.land_uses.names, you could add an extension to the association to do what you want.

    class Photo
      has_and_belong_to_many :land_uses do
        def names_as_list_string
          all.map(:name).join ', '
        end
      end
    end
    

    For more information on association extensions, check out the docs.

    The best way to conform to the law of demeter is to do more or less what you are doing though, because by adding your method on Photo, it means that the methods that interact with Photo, don't also need to know about the LandUse class, just that photo has a method that returns a string of the names of land uses.