Follow Law of Demeter when using collections?

Question

Within Ruby on Rails (or any other language with a collection...) is it necessary to break Law of Demeter violations up when querying something simple like a count?

class Survey
  has_one :kingdom

  def first_survey?
    # Should this be broken according to Law of Demeter?
    self.kingdom.surveys.count == 1
    # -or-
    self.kingdom.surveys_count == 1
  end
end

class Kingdom
  has_many :surveys

  # Is this the "right thing to do", or overkill?
  def surveys_count
    self.surveys.count
  end
end

Answer 1

Usually when I see Law of Demeter violations the first question I ask isn't "How do I 'avoid a dot'?" The question you should ask is "Does this functionality that violates belong to someone else?" In this case I would probably structure it like so:

class Survey
  belongs_to :kingdom
end

class Kingdom
  has_many :surveys

  def first_survey?(survey)
    surveys.first == survey
  end
end

kingdom = Kingdom.find(kingdom_id)
first_survey = kingdom.surveys.first
last_survey = kingdom.surveys.last # Assuming there is more than one survey.

kingdom.first_survey?(first_survey) #=> true
kingdom.first_survey?(last_survey)  #=> false

By structuring it this way the Survey no longer has to reach into the Kingdom object and query its assocations, avoiding the Law of Demeter violation. This also allows you to change the definition of first_survey? later on. A contrived example would be if Surveys could be "published". The first_survey? for a Kingdom could then be easily changed to support only checking if the passed in Survey is the first Survey with the published attribute set to true.

Answer 2

Definition from Wikipedia

More formally, the Law of Demeter for functions requires that a method m of an object O may only invoke the methods of the following kinds of objects

1. O itself  
2. m's parameters  
3. Any objects created/instantiated within m  
4. O's direct component objects  
5. A global variable, accessible by O, in the scope of m

In particular, an object should avoid invoking methods of a member object returned by another method. For many modern object oriented languages that use a dot as field identifier, the law can be stated simply as "use only one dot". That is, the code a.b.Method() breaks the law where a.Method() does not. As a simple example, when one wants a dog to walk, one would not command the dog's legs to walk directly; instead one commands the dog which then commands its own legs.

In the case above yes this would break the Law of Demeter:

 def first_survey?
    # Should this be broken according to Law of Demeter?
    self.kingdom.surveys.count == 1

As to whether it is overkill, I say no but than again I don't like bad code (that is code that is incorrect and allows for more errors than it should, many an argument I get into over it).

What you would want to do is something like this:

def first_survey?
    self.kingdom.surveys_count == 1

If my understanding is correct, this does not violate Demeter, because you are not accessing the suverys.count property directly. Instead you are asking the kingdom to give you back an internal property it has (this is more correct in my opinion as it is utilizing the API of the object kingdom)

# Is this the "right thing to do", or overkill?
  def surveys_count
    self.surveys.count
  end

This is the "right thing to do" how else wou;ld external systems access that property?

From the comments

use only one dot is actually a very bad rule since "string".strip.downcase.tr_s('^[a-z0-9]', '-') is not a violation of demeter

Well since you brought it up, I will counter your statement. It actually isn't a bad rule because you overlooked a very critical piece of information here, the strip function returns a new string object as well as downcase as does tr_s. If you are generating three extra objects, I would be very concerned for any developer seeing that. And it really does violate the rule, because you are accessing a member directly. What you would want to consider, and by no means am I a Ruby expert is something like this:

class MyString  
{  
     string internal_string;
         def strip()  
         {  
              self.internal_string = self.internal_string.strip
         }  

       def downcase()
       {
         self.internal_string = self.internal_string.downcase
       }  

       def tr_s()
       {  
           self.internal_string = self.internal_string.tr_s
       }  
}  

What the above does is allows developers to invoke your API by putting a thin wrapper around the library functions. Some may call this overkill, but it gives very clear lines as to what can and should be done. Now to make this Demeter approved (if I understand the law correctly)

myString.strip().downcase().tr_s()

Now what may not be clear, this does not violate Demeter as I am invoking the API that is exposed for myString, overkill (maybe), most correct (almost definitely).