0
votes

I really want to start learning Rails best practices, especially following the "fat model, skinny controller" logic.

Say I have the following comment controller

class CommentsController < ApplicationController

    def create
        @post = Post.find(params[:post_id])
        @comment = @post.comments.create(comment_params)
        @comment.user_id = current_user.id if current_user
        @comment.save!

        if @comment.save
            redirect_to post_path(@post)
        else
            render 'new'
        end
    end

    def edit
        @post = Post.find(params[:post_id])
        @comment = @post.comments.find(params[:id])
    end

    def update
        @post = Post.find(params[:post_id])
        @comment = @post.comments.find(params[:id])

        if @comment.update(params[:comment].permit(:comment))
            redirect_to post_path(@post)
        else
            render 'Edit'
        end
    end

    def destroy
        @post = Post.find(params[:post_id])
        @comment = @post.comments.find(params[:id])
        @comment.destroy
        redirect_to post_path(@post)
    end

    private

    def comment_params
        params.require(:comment).permit(:comment)
    end

What's a good place to start refactoring the code? Immediately I think I an make the @post and @comment in both edit and update into a separate method, follow by calling a before_action on the method. But that is still putting all the code in the controller.

Are there any code that I can move to the model? If so, how should I structure them?

2
You should try to search for relevant articles on the fat model approach -- it's not something that can (or should) be covered in a couple of paragraphs on SO. Speaking very generally, "fat model" means that you put your business logic in the model -- it does not mean that you put all logic in the model. The only part of your code that is begging to be refactored is the fact that you call @comment.save! (a forced save) followed by if @comment.save (which is a second attempt to save the same comment you already saved. You're begging to fail any unique validations. - MarsAtomic
@MarsAtomic actually a double save won't create a duplicate record, but yea it's an extra unneeded save. - Mohammad AbuShady

2 Answers

1
votes

This code doesn't have much room for improvement, it's a basic crud, here's an example of a before_action like you suggested

before_action :load_post_and_comment, only: %i(edit update destroy)

def load_post_and_comment
  @post = Post.find(params[:post_id])
  @comment = @post.comments.find(params[:id])
end

And here a couple of other notes

def create
  # ...
  @comment.save!
  if @comment.save
    # ...
  else
    # ..
  end
end

In this codition the you should remove the extra @comment.save! you only need to save once.

def update
  # ...
  if @comment.update(params[:comment].permit(:comment))
    # ...
  else
    # ...
  end
end

You already have the comment_params method, use it, because if you at any point add a new attribute to the comment, you'll update that method but you'll probably forget this part and you'll get werid errors till you notice that you need to permit here too.

0
votes

If you want to really go all out with the skinny controller model, there is this gem: https://github.com/NullVoxPopuli/skinny_controllers

Where, you'd configure your CommentsController like so:

class CommentsController < ApplicationController
  include SkinnyControllers::Diet

  def create
    if model.errors.present?
      render 'new'
    else
      redirect_to post_path(model)
    end
  end

  def update
    redirect_to post_path(model)
  end

  # ... etc

  private

  def comment_params
    params.require(:comment).permit(:comment)
  end

end