4

I want to follow the Law of Demeter. As I am going through my code searching for the "two dots", I find myself asking if it's really worth setting up the delegation responsibilities in this type of context:

FROM

class Activity
  def increment_user_points!
    self.user.increment_points!(points, true)
  end
end

TO

module UserDelegator
  def user_increment_points!(points, increment_credits=false)
    self.user.increment_points!(points, increment_credits)
  end
end

class Activity
  include UserDelegator

  def increment_user_points!
    user_increment_points!(points, true)
  end
end

What are your thoughts?

keruilin
  • 16,782
  • 34
  • 108
  • 175
  • The reason you've got two dots here is because of the explicit declaration of `self` as the receiver. You can get rid of that, because `self` is the default receiver. – Wayne Conrad Oct 28 '12 at 13:47

2 Answers2

2

Your example is not breaking the Law of Demeter. The user is an attribute of the activity and you are accessing a public API method on the user, so you are not in error with the original implementation.

The goal of the Law of Demeter is to avoid breaking object encapsulation. Your "two dots" approach is somewhat of an oversimplification of the idea. In reality, you should examine how your objects are interacting and ensure that you are not reading too far into the attributes or relationships of other objects. For instance, if the following would be a violation of the rule:

def update_user_address
  @user.address.set(new_address)
end

This would be due to the fact that the address is the business of the user, and it should encapsulate access to it appropriately through the user's API. As a client of the user, we should never directly have access to the API's of the users attributes.

Again, in your example, you are using the users API directly, which is fine and is not violating Demeters Law. All that said, I have found the general rule to be a good one to follow. Your code will generally be easier to change and maintain and classes will be easier to refactor if you avoid breaking object encapsulation as shown.

Pete
  • 17,885
  • 4
  • 32
  • 30
1

I'd actually expect TO to look more like this:

class User
  def increment_points!(points, increment_credits)
    @points+=points if increment_credits
    @points-=points unless increment_credits
  end
end

class Activity
  def increment_user_points!
    @user.increment_points!(points, true)
  end
end

Creating a module to include would seem to create more complexity. And the whole point of the Law of Demeter (I like to think of it more as a guideline..) is that you ought to be able to do whatever you like to User's internals without having to rewrite much code outside the class. Your UserDelegator module doesn't help much -- you still get to re-write that code when you fiddle with User's internals.

But if it were me, I don't think I'd even bother with this, unless you're finding yourself rewriting a lot of code to make simple changes to User. Maybe that's just because I'm used to the Linux kernel coding style, which breaks the law of Demeter on a regular basis:

static inline int need_reval_dot(struct dentry *dentry)
{
    if (likely(!(dentry->d_flags & DCACHE_OP_REVALIDATE)))
        return 0;

    if (likely(!(dentry->d_sb->s_type->fs_flags & FS_REVAL_DOT)))
        return 0;

    return 1;
}

Three objects away :) and I'm not sure the code would be more legible if written:

need_reval_dot(dentry) {
    if(likely(!dentry_need_reval_dot(dentry))
        return 0;
}

dentry_need_reval_dot(dentry) {
    return superblock_need_reval_dot(dentry->d_sb);
}

superblock_need_reval_dot(sb) {
    return fs_type_need_reval_dot(sb->s_type);
}

fs_type_need_reval_dot(s_type) {
    return fs_flags_need_reval_dot(s_type->fs_flags);
}

fs_flags_need_reval_dot(fs_flags) {
    return fs_flags & FS_REVAL_DOT;
}

So I'm all in favor of following guidelines in moderation -- ask yourself if your modifications actually lead to cleaner, more maintainable code, or if it is just following a rule for the sake of following rules.

sarnold
  • 102,305
  • 22
  • 181
  • 238