Skip to content

Dan Meehan Solution#37

Open
dmee3 wants to merge 1 commit into
paircolumbus:masterfrom
dmee3:master
Open

Dan Meehan Solution#37
dmee3 wants to merge 1 commit into
paircolumbus:masterfrom
dmee3:master

Conversation

@dmee3
Copy link
Copy Markdown

@dmee3 dmee3 commented May 23, 2017

Comment thread associations.rb
def booked_guests
[].tap do |r|
self.bookings.each { |b| r << b.guest }
end
Copy link
Copy Markdown

@sjreich sjreich May 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simpler code: self.bookings.map { |booking| booking.guest }. And actually self is redundant here.

But more importantly, this is going to generate a separate query for each guest (an "n+1" problem).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good point - I didn't think about function performance while I was adding to the models.
The User.booked_rooms method will run into the same problem then, right?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ActiveRecord's has_many :through association can take care of this for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants