How to handle two-way dependencies in modularized codebase using `belongs_to` / `has_many` and ActiveSupport hooks

System Design
21

Message excluded from import.

22

Message originally sent by slack user U723PSM8PD2

That’s absolutely fair. I’m more interested in hearing about what people think would go wrong (concretely, not conceptually) if we do go ahead with it in a more widespread way, but it’s still a valuable perspective.

23

Message excluded from import.

24

Message originally sent by slack user U723PSM8PD2

:smile:

25

Very interesting discussion, guys. I can see <@U71102MDOF5>’s super valuable points… but I can also relate to <@U723PSM8PD2> as I’m on a similar spot myself :sweat_smile:.

26

<@U723PSM8PD2> IMHO your proposed 3rd stage is kinda-ok as long as you guys really do that only temporarily. At some point you guys will have to replace the code generating those N+1s on the 2nd stage, with some query objects or similar abstractions, as Aaron mentioned. But I can see your need for an intermediate version…

27

The only benefit of that 3rd, more convoluted version, is that the code in the companies pack doesn’t have employees mentioned.

28

That is progress, IMHO. But shouldn’t be enough.

29

Message excluded from import.

30

You might end up tricking yourself for longer than desired… hiding the dust under the carpet.

31

Message excluded from import.

32

Message excluded from import.

33

Message excluded from import.

34

One more thing I want to mention as it looks very related… I have been using a slightly-different version of the 2nd state where I don’t have a fix for the N+1s the inverse_of offers, but I do have a workaround for the benefits of a dependent: x. Before you say it, <@U71102MDOF5>, yeah, I realize this could go into a more specific object responsible for the whole thing.

Continuing with <@U723PSM8PD2>'s example this would be my version:

# engines/companies/app/models/company.rb
class Company < ApplicationRecord
end

# engines/employees/app/models/employee.rb
class Employee < ApplicationRecord
  belongs_to :company

  scope :for_company, ->(o) { where(company_id: o) }
end

# engines/employees/config/initializers/init.rb
Rails.application.configure do
  config.after_initialize do
    # This does the same as `dependent: :destroy_all` on a `has_many`
    # but avoiding the `has_many` to invert the dependency
    Company.before_destroy(->(o) { Employee.for_company(o).destroy_all })
  end
end
35

Message excluded from import.

36

Message excluded from import.

37

well… I think it depends. There could be a lot of different ways to accomplish the same. Why not do it at the DB layer instead, for example? DBs are capable of deleting in cascade.

38

I think each case might be different.

39

There’s pros and cons for putting it at different levels of the stack.

40

Message excluded from import.