class Post < ApplicationRecord
has_many :comments
end
class User < ApplicationRecord
has_many :comments
end
class Comment < ApplicationRecord
belongs_to :post
belongs_to :user
end
class PostsController < ApplicationController
def show
@post = Post.where("published = true").where("id = #{params[:id]}").first
@post ||= Post.last
end
end
<% @post.comments.each do |comment| %>
<p> <b><%= comment.user.name %> wrote a comment:</b>
<%= comment.text %>
</p>
<% end %>
The intention of this code is to render a post and its comments. With this code in mind, please answer the following:
- Write a short paragraph describing what is happening in the show action of PostsController (consider both the controller action and the view).
- How would you improve the existing code? Please show what changes you would make and describe why.
- Controller is filling an instance variable, that instance variable will be available to the rendered erb.
In order to fill up @post it makes a db query using active record api.
select posts.* from posts where published = true and id = 1234
That query will return a ActiveRecordQuery (ActiveRecord::Relation) object which will be resolved when.first
is executed.
If the post is not found it will get the last created post from the database.
The view will execute @post.comments
that will perform another query
select coments.* from comments where post_id = 1234
the result will be converted into an array and it will start iterating over that result.
For each comment it will perform comment.user.name
Each time this code executes a new query will be executed.
select users.* from users where comment_id = 9876
So in case the user have one post with 100 comments.
This is performing
- 1 query for fetching the post
- 1 query for fetching the comments
- 100 queries for fetching the users
- This can be avoided by providing the view with a structure of related objects like this
Post -*> Comments --> User
You can perform only one query.
select *.user, *.posts, *.comments
from comments
inner join posts on comments.posts_id = posts.id
inner join users on comments.user_id = users.id
where posts.publisehd = true and posts.id = zzzz
The way to tell rails to cache all the relations is by using includes
instead of joins
.
@comments = Comment.includes(:users).includes(:posts).where(posts: { id: zzzz, published: true }).to_a
(I haven't tried this code and probably it doesn't work but it is just a guide to the solution)
The view would change a little bit:
<% @comments.each do |comment| %>
<p> <b><%= comment.user.name %> wrote a comment:</b>
<%= comment.text %>
</p>
<% end %>
The execution of comment.user.name
won't perform any new query to the database because includes
loaded the objects on memory.