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

Question

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?

Answer 1

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.

Answer 2

I am not in front of a rails app but I believe

photo.land_uses

with return an Array of LandUse objects

So you just need to move your map down to that array like:

photo.land_uses.map(&:name).join(', ')

which is what you had originally - just in your other model. I think you may be right and it means that Photo knows too much about LandUse therefore I would move it out.

Answer 3

You can use :

class LandUse
  def self.list_for_photo(id)
    LandUse.find_by_photo_id(id).join(', ')
  end

  def to_s
    self.name
  end
end

Hope it helps !