This message was imported from the Ruby/Rails Modularity Slack server. Find more info in the import thread.
Message originally sent by slack user U71TN2WF04X
Hello I am a bit lost on solving a case of dependencies, I read here about the subject but I am not getting the solution.
My case is the following. I have two packs: users and jobs
In users I have the User class
class User < ApplicationRecord
belongs_to: :job, inverse_of: :users, optional: true
...
end
In jobs I have the Job class
class Job < ApplicationRecord
has_many: :users, inverse_of: :job
...
end
The jobs pack list as a dependency the users pack. This causes a violation in the User class since it references job , is this something I am supposed to just ignore? How can I identify those violations from others where I would be explicitly using the Job class like this one?
class User < ApplicationRecord
def self.all_jobs
Jobs.all
end
end
My User class is very big so its hard to see these violations
This question will easily blow my ability to communicate well via slack out of the water. I see two big points:
• The circular dependency between job and user is always going to mean that you have some sort of violation if you keep it. There are two ways out: the two classes are actually super closely connected… then they can live in the same package. Or, you delete one of the association directions…
• Without further context, the way I would propose to do that is to get rid of the has_many (I feel like half of my talks hate on has_many at some point. In fact, a buddy and “upgraded” has_many in our gem https://github.com/dam5s/active_has_many )
• In this specific example you are also identifying User as a god object (“My User class is very big” - the other half of my talks is about god objects) and you always want to shrink the size of those objects. This will mean that any access of jobs’ users will go through Job. I.e., something like
class Job < ApplicationRecord
def self.all_for_user(user_id)
Jobs.where(user_id: user_id)
end
end
Thank you both! I hate the fact that this class is so big
I hope to convince my team that modularization can help us shrink classes to actually do what they are designed to do instead of adding any related functionality to it!
If you just want to not have all the logic in one file consider splitting the model into separate concerns. It doesn’t help with separating into packs, but it can at least make the class less monolithic https://dev.37signals.com/vanilla-rails-is-plenty/
<@U712YWCKK8T> without making this thread much longer. Using mixins in classes (especially in the really big ones…) has, in my experience, only one effect: Making the class effectively even bigger while hiding its complexity away and making more brittle code, because who really knows how multiple mixins interact… Concerns don’t change anything about the fundamentals, they just, unfortunately, make mixins easier to construct
chiming in in an old thread here. Great writeup, Scott. One question that comes to mind, when replacing has_many with methods like all_for_user is how do you replace the functionality of other has_many options like dependent: :restrict_with_exception and so on? I assume you still have the foreign keys defined in the DB and then rescue and react to those instead of leveraging Rails’ built-in checks?