current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite
1

I have a RoR 4 app, and its model have these associations:

has_many :accreditations
has_one :legal, -> { where(kind_cd: Accreditation.legal) }, class_name: 'Accreditation'
has_many :departments, -> { where(kind_cd: Accreditation.department) }, class_name: 'Accreditation'

So, you see that these associations similar, but legal & departments have more conditions than accreditations. Can I replace class_name: 'Accreditation' with something like using :accreditations?

share|improve this question
add comment

2 Answers

up vote 3 down vote accepted

I don't see what you will gain with your hypothetical using syntax, let's say there is one... your code will look like this:

has_many :accreditations
has_one :legal, -> { where(kind_cd: Accreditation.legal) }, using: :accreditations
has_many :departments, -> { where(kind_cd: Accreditation.department) }, using: :accreditations

How is it better than your current code? It is not more expressive, nor is it more succinct, nor DRY.

Also, these associations are similar to some extent, but they look as DRY as you can make them - one is has_one, while the others are has_many, one uses the default idiom, while the others have names different than the associated class, conditions are different (and though one might argue you can predict the filter by the association name, one is singular, and the other is plural...)

In short, I think your current code is good enough - any change will only harm readability.

share|improve this answer
    
Uri, but if I make two queries like @accreditations = @contractor.accreditations;@legal = @contractor.accreditations I'll get second query faster because it will use cache from first query, isn't it? –  asiniy Mar 27 at 5:31
add comment
up vote 4 down vote

There are two ways to tackle this situation. One of them is what you did: follow the Law of Demeter; I don't have any advice to improve your code. The second way is move the logic to the other model. In that case you'd write:

class Company
  has_many :accreditations
end

class Accreditation
  scope :departments, where(kind_cd: department)

  def self.legal
    where(kind_cd: legal).first
  end
end

Both are ok even though I prefer the second because the logic of departments/legal is within the accreditation model.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.