Search code examples
ruby-on-railsarraysrubyperformanceeager-loading

Includes method ( n+ 1 issue ) doesn't work with a push method but does with += when assigning to an array


Consider the simple relation between Employee and Company models(many to many):

Company model:

has_many :employees, through: :company_employees
has_many :company_employees

Employee model:

has_many :companies, through: :company_employees
has_many :company_employees

CompanyEmployee model(join table):

belongs_to :employee
belongs_to :company

also an Owner model:

has_many :companies

So in my system i have an owner which may have several companies and an Employee, which may work for multiple companies.

Now, In my employees controller i want to fetch all of the employees workin for an owner:

def owners_linked
 @company_employees = []
 owner.companies.each do |company|
   @company_employees.push (company.company_employees.includes(:company, :employee)) # when += instead of push - it works
 end
 respond_to do |format|
   format.js {render "employees_list"}
 end
end

I need to have an access to Employee instances(personal data), company_employees table (information about the position in the company) and Company(company related data).
To resolve n+1 problem and speed up the performance i use includes method. Well, the problem is that in my controller action in line:

@company_employees.push company.company_employees.includes(:company, :employee)

when using push method it doesn't work. I obtain the error in the view that employee method is not defined. On the other hand when i change the push to += sign it works perfectly fine.

Can anyone help me to understand why it's like so?
I know that += is inefficint so i'd rather not stick to it.


Solution

  • This doesn't have anything to do with your use of includes.

    When you use += you end up with an array of CompanyEmployee objects. However when you use push you are no longer concatenating arrays but creating an array of collections. You are then calling employee on the collection rather than an element of the collection which is why you get an error.

    Personally I would write this as

    @company_employees = owner.companies.flat_map do |company|
      company.companee_employees.include(...)
    end
    

    Although I would do so for reasons of succinctness rather than performance. Any performance difference between += and other ways of concatenating arrays is minuscule compared to the time it takes to fetch data from the database.

    This doesn't entirely solve your n+1 problem though, since the data for each company is loaded separately. I would do

    @company_employees = owner.companies.include(company_employees: [:company, :employee]).flat_map(&:company_employees)
    

    Which doesn't do as many queries.