ajk4550 ajk4550 - 6 months ago 33
Ruby Question

DRYing this Ruby code

Recently, I've started using Rubocop and have been trying to think better about my code and if I can be written better. I have a create and update method that are VERY similar. Rubocop is complaining that the method has too many lines of code [12/10]. I am wondering how you would go about following the DRY principle here. It seems to me that respond_to should be brought to its own private method. But I can't figure out what would be the best way to do that since:


  1. The flash can be a :success or :danger

  2. One checks if the models saved and the other if it updated.

  3. Different rendering depending on if the model saved or if it had an error



I also don't know if I should just be leaving it alone. The fact that it is so redundant is really getting to me though. Ultimate I want to have the cleanest code, I am just not sure if I should DRY this method

def create
@category = Category.new(category_params)

respond_to do |format|
if @category.save
flash[:success] = 'Category Successfully Created'
format.html { redirect_to admin_category_path(@category) }
format.json { render :show, status: :created, location: @category }
else
flash[:danger] = 'Errors in creating category, see below'
format.html { render :new }
format.json { render json: @category.errors, status: :unprocessable_entity }
end
end
end

def update
@category = Category.find(params[:id])

respond_to do |format|
if @category.update(category_params)
flash[:success] = 'Category Successfully updated!'
format.html { redirect_to admin_category_path(@category) }
format.json { render :show, status: :created, location: @category }
else
flash[:danger] = 'Errors in updating category, missing information'
format.html { redirect_to action: 'edit', id: @category.id }
format.json { render json: @category.errors, status: :unprocessable_entity }
end
end
end

Answer

It doesn't make any sense to try to make a single method from create and update, as they serve two very different purposes.

Instead, you could consider the following things:

  • do you really need the json format? If you are not using it, you can safely remove those lines;
  • remove @category = Category.find(params[:id]) from update and move it to a method in before_action

    before_action :find_category, only: [:edit, :update]
    
    def find_category
      @category = Category.find(params[:id])
    end
    
  • last but not least, Rubocop doesn't always have the right answer: focus on clearness!