Trouble with dependencies between two packs and identifying violations in a large class

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 :wave: 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 :wink:)
• 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

Message originally sent by slack user U71V6G2PDJS

https://rubymod.slack.com/archives/C0164JN9H5F/p1672962559742419?thread_ts=1672953197.022149&cid=C0164JN9H5F

Message originally sent by slack user U71TN2WF04X

Thank you both! I hate the fact that this class is so big :cry:
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!

Message originally sent by slack user U712YWCKK8T

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/

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

Message excluded from import.

<@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

Message excluded from import.

Message excluded from import.

Message excluded from import.

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?

Message excluded from import.