Law of Demeter - Question

Had a question about how to refactor some of my code. I've been reading some books AntiPatterns & Refactoring. Given the code block here, should the method here be in Evaluation or Skill? If I'm following the "rule" in AntiPatterns for using only one dot, and reports is knowing too much about skills and evaluations... how would this look like if I wanted to retrieve the accuracy evaluation from my report object?

class Report < ApplicationRecord
  has_many :evaluations
  def accuracy
    evaluations.joins(:skill).where('skills.code = (?)', 'ACC').try(:first)
  end
end

class Evaluation < ApplicationRecord
  belongs_to :report
  belongs_to :skill
end

class Skill < ApplicationRecord
  has_many :evaluations
end

I've never really agreed with the single method call (dot) advice, personally.

One thing, you don't need the .try because where will always return an ActiveRecord::Relation object which will always respond to first. It may or may not return a record, so anything you call after that would need to check if accuracy was nil.

A little cleaner version is to use find_by

# Returns an Evaluation
def accuracy
  evaluations.joins(:skill).find_by(skills: { code: 'ACC' })
end

Thanks! I appreciate that note - it is cleaner. As I'm tryng to apply it to the single method call I've come up with this, but it's currently not giving me the same result as the older code.

class Report < ApplicationRecord
  has_many :evaluations

  def accuracy
    evaluations.accuracy
  end
end

class Evaluation < ApplicationRecord
  belongs_to :report
  belongs_to :skill

 delegate :accuracy, to: :skill

end

class Skill < ApplicationRecord
  has_many :evaluations

 def self.accuracy
   find_by(code: "ACC")
 end

end

Yeah, that is not equivalent there because you're not filtering the evalulations, only the skill. You need to have it in a single query.

DHH doesn't like the Law of Demeter, so I wouldn't worry too much about trying to follow it. He's written about it a few times: https://signalvnoise.com/posts/3341-pattern-vision

Thanks that's very helpful I'll read it. Perhaps I'll go with the follow that seems to be inbetween the two?

I'm going to think on your version and this one a little more. Try and work out the pros/cons to placing them in both of their places. Thanks again!

class Report < ApplicationRecord
  has_many :evaluations

  def accuracy
    evaluations.accuracy
  end
end

class Evaluation < ApplicationRecord
  belongs_to :report
  belongs_to :skill

  def self.accuracy
    joins(:skill).find_by(skills: { code: "ACC" })
  end
end

class Skill < ApplicationRecord
  has_many :evaluations
end