Now that I've got some scaffolded code in there for the models I intend to use for this application there is something I can do to DRY up the code somewhat.

Looking at CodeClimate for my repository as it stands right now it has given me a rating of C, with 28 instances of duplication.

So looking at what CodeClimate has flagged as an issue shows this:

All this controller code is the same with the only thing changing being the name of the class and instance variables. The same thing is happening in the update action and all other actions as well. I can DRY this up by introducing a concern which refers to something a bit more generic.

I'll call a specific instance of something a resource and a collection of them a collection.

The concern would look like this:

app\controllers\concerns\resources.rb

# DRY up controllers by using "resource" for a single record, "collection" for all records
module Resources
  extend ActiveSupport::Concern

  included do
    before_action :set_resource, only: %i[show edit update destroy]
    before_action :set_collection, only: %i[index]
  end

  private

  def set_collection
    @collection = resource_class.all
  end

  def set_resource
    @resource = resource_class.find(params[:id])
  end

  def resource_class
    # eg ItemSellPacksController -> ItemSellPack
    @resource_class ||= controller_name.gsub('Controller', '').singularize.classify.safe_constantize
  end

  def resource_name
    # eg ItemSellPack -> item_sell_pack
    @resource_name ||= resource_class.name.underscore.to_sym
  end

  def resource_human_name
    # eg ItemSellPack -> Item sell pack
    @resource_human_name ||= resource_class.model_name.human
  end
end

Now I can change my controllers from this:

class BrandsController < ApplicationController
  before_action :set_brand, only: %i[show edit update destroy]

  # GET /brands or /brands.json
  def index
    @brands = Brand.all
  end

  # GET /brands/1 or /brands/1.json
  def show; end

  # GET /brands/new
  def new
    @brand = Brand.new
  end

  # GET /brands/1/edit
  def edit; end

  # POST /brands or /brands.json
  def create
    @brand = Brand.new(brand_params)

    respond_to do |format|
      if @brand.save
        format.html { redirect_to(brand_url(@brand), notice: 'Brand was successfully created.') }
        format.json { render(:show, status: :created, location: @brand) }
      else
        format.html { render(:new, status: :unprocessable_entity) }
        format.json { render(json: @brand.errors, status: :unprocessable_entity) }
      end
    end
  end

  # PATCH/PUT /brands/1 or /brands/1.json
  def update
    respond_to do |format|
      if @brand.update(brand_params)
        format.html { redirect_to(brand_url(@brand), notice: 'Brand was successfully updated.') }
        format.json { render(:show, status: :ok, location: @brand) }
      else
        format.html { render(:edit, status: :unprocessable_entity) }
        format.json { render(json: @brand.errors, status: :unprocessable_entity) }
      end
    end
  end

  # DELETE /brands/1 or /brands/1.json
  def destroy
    @brand.destroy!

    respond_to do |format|
      format.html { redirect_to(brands_url, notice: 'Brand was successfully destroyed.') }
      format.json { head(:no_content) }
    end
  end

  private

  # Use callbacks to share common setup or constraints between actions.
  def set_brand
    @brand = Brand.find(params[:id])
  end

  # Only allow a list of trusted parameters through.
  def brand_params
    params.require(:brand).permit(:name, :canonical, :count)
  end
end

to this:

class BrandsController < ApplicationController
  include Resources

  # GET /brands or /brands.json
  def index; end

  # GET /brands/1 or /brands/1.json
  def show; end

  # GET /brands/new
  def new
    @resource = resource_class.new
  end

  # GET /brands/1/edit
  def edit; end

  # POST /brands or /brands.json
  def create
    @resource = resource_class.new(brand_params)

    respond_to do |format|
      if @resource.save
        format.html { redirect_to(brand_url(@resource), notice: 'Brand was successfully created.') }
        format.json { render(:show, status: :created, location: @resource) }
      else
        format.html { render(:new, status: :unprocessable_entity) }
        format.json { render(json: @resource.errors, status: :unprocessable_entity) }
      end
    end
  end

  # PATCH/PUT /brands/1 or /brands/1.json
  def update
    respond_to do |format|
      if @resource.update(brand_params)
        format.html { redirect_to(brand_url(@resource), notice: 'Brand was successfully updated.') }
        format.json { render(:show, status: :ok, location: @resource) }
      else
        format.html { render(:edit, status: :unprocessable_entity) }
        format.json { render(json: @resource.errors, status: :unprocessable_entity) }
      end
    end
  end

  # DELETE /brands/1 or /brands/1.json
  def destroy
    @resource.destroy!

    respond_to do |format|
      format.html { redirect_to(brands_url, notice: 'Brand was successfully destroyed.') }
      format.json { head(:no_content) }
    end
  end

  private

  # Only allow a list of trusted parameters through.
  def brand_params
    params.require(:brand).permit(:name, :canonical, :count)
  end
end

There is still some more I can do here, notice how I now have some empty methods (like index and show) and others that are still exactly the same no matter which controller it is in. The create / update / destroy methods now only differ in the URLs or text they include.

I'll address these issues in a moment.

Now since I no longer have @brand or @brands only @resource and @collection the views need to be modified as well.

This edit.html.erb goes from:

<div class="mx-auto md:w-2/3 w-full">
  <h1 class="font-bold text-4xl">Editing brand</h1>

  <%= render "form", brand: @brand %>

  <%= link_to "Show this brand", @brand, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
  <%= link_to "Back to brands", brands_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
</div>

to this:

<div class="mx-auto md:w-2/3 w-full">
  <h1 class="font-bold text-4xl">Editing brand</h1>

  <%= render "form", brand: @resource %>

  <%= link_to "Show this brand", @resource, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
  <%= link_to "Back to brands", brands_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
</div>

Now back to reducing the duplication in the controller actions I'll add another concern that looks like this:

app\controllers\concerns\actions.rb

# DRY up controllers by extracting generic action methods that can be overridden if necessary
module Actions
  extend ActiveSupport::Concern

  # GET /collection or /collection.json
  def index; end

  # GET /collection/1 or /collection/1.json
  def show; end

  # GET /collection/new
  def new
    @resource = resource_class.new
  end

  # GET /collection/1/edit
  def edit; end

  # POST /collection or /collection.json
  def create
    @resource = resource_class.new(resource_params)

    respond_to do |format|
      if @resource.save
        format.html { redirect_to(resource_url(@resource), notice: "#{resource_human_name} was successfully created.") }
        format.json { render(:show, status: :created, location: @resource) }
      else
        format.html { render(:new, status: :unprocessable_entity) }
        format.json { render(json: @resource.errors, status: :unprocessable_entity) }
      end
    end
  end

  # PATCH/PUT /collection/1 or /collection/1.json
  def update
    respond_to do |format|
      if @resource.update(resource_params)
        format.html { redirect_to(resource_url(@resource), notice: "#{resource_human_name} was successfully updated.") }
        format.json { render(:show, status: :ok, location: @resource) }
      else
        format.html { render(:edit, status: :unprocessable_entity) }
        format.json { render(json: @resource.errors, status: :unprocessable_entity) }
      end
    end
  end

  # DELETE /collection/1 or /collection/1.json
  def destroy
    @resource.destroy!

    respond_to do |format|
      format.html { redirect_to(collection_url, notice: "#{resource_human_name} was successfully destroyed.") }
      format.json { head(:no_content) }
    end
  end
end

To support DRYing up create, update and destroy there are some new methods that have been added: resource_params - These are the permitted parameters the controller will accept resource_url - The URL to a single instance of a resource collection_url - The URL to a collection of class of resource

Here are the concerns that implement these:

app\controllers\concerns\urls.rb

# DRY up controllers by extracting generic resource URL usage
module Urls
  extend ActiveSupport::Concern

  private

  def resource_url(resource)
    polymorphic_url(resource)
  end

  def collection_url
    polymorphic_url(resource_class)
  end
end

app\controllers\concerns\parameters.rb

# DRY up controllers by extracting generic parameter processing
module Parameters
  extend ActiveSupport::Concern

  private

  def resource_params
    params.require(resource_name).permit(permitted_params)
  end

  def permitted_params
    []
  end
end

Now my controller will look like this:

class BrandsController < ApplicationController
  include Resources
  include Actions
  include Urls
  include Parameters

  private

  # Only allow a list of trusted parameters through.
  def permitted_params
    %i(name canonical count)
  end
end

There is one last thing I can do now as all my controllers are going to want to use those inclusions. I'll add a new controller class called ResourcesController and inherit that instead.

app\controllers\resources_controller.rb

class ResourcesController < ApplicationController
  include Resources
  include Actions
  include Urls
  include Parameters
end

Which means my controller looks like this:

class BrandsController < ResourcesController
  private

  # Only allow a list of trusted parameters through.
  def permitted_params
    %i(name canonical count)
  end
end

I can now execute all my tests to confirm the controller still behaves correctly.

# Running:

.......DEBUGGER: Attaching after process 8708 fork to child process 8812
Capybara starting Puma...
* Version 5.6.4 , codename: Birdie's Version
* Min threads: 0, max threads: 4
* Listening on http://127.0.0.1:57978
...................................................................................................................................................

Finished in 25.379320s, 6.0679 runs/s, 7.1712 assertions/s.
154 runs, 182 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for Unit Tests to /Users/andrewfoster/rails/catalogue_cleanser/coverage. 494 / 565 LOC (87.43%) covered.

And now I'll have a look at what CodeClimate thinks of the result:

Much better!