Code refactory on a small controller (thin controller model)

I have an action with a controller that makes a list of products, pagination and some filters, for example a category (from the drop-down list), a title (from a text field), a stock (from a checkbox) This is my controller:

  class ProductsController < ApplicationController
  def index
    @products = Product.where(active:1).where("title LIKE ?","%#{params[:title]}%")
      if params[:stock]
        @products=@products.where("stock = 0")
      end
      if params[:category] 
        @products=@products.where("category_id LIKE ?","#{params[:category]}")
      end
    @products= @products.paginate(:page => params[:page])
    @categories= Category.all
  end

And my model:

class Product < ActiveRecord::Base
  belongs_to :category
    ...some validations...
end

What can I change to make my controller thinner? Thanks

+4
source share
4 answers

Model

class Product < ActiveRecord:::Base
  scope :active, where(active: 1)

  def self.with_stock(stock=nil)
    return where(stock: 0) if stock
    self
  end

  def self.categorized(category=nil)
    return self.where(category: category) if category
    self
  end

  def self.titled(title=nil)
    return self.where("title LIKE ?", 'title') if title
    self
  end

  def self.list(params)
    title    = params[:title]
    category = params[:category]
    page = params[:page]
    self.titled(title).with_stock(stock).categorized(category)
      .paginate(page).active
  end
end

controller

 def index
   @products = Product.list(params)
 end

Do not send a category to the controller. Do it in a template / partial. ONE only from the controller.

+2
source

I suggest a specific refactoring style:

controller

class ProductsController < ApplicationController
   def index
      @products = Product.titled params[:title]
      @products = @products.in_stock if params[:stock]
      @products = @products.category params[:category] if params[:category]

      @products = @products.paginate :page => params[:page]
      @categories = Category.all
   end
end

model

class Product < ActiveRecord::Base
   belongs_to :category
   ...
   scope :titled, proc {| title | where(active:1).where("title LIKE ?","%#{title}%")
   scope :in_stock, proc { where("stock = 0") }
   scope :category, proc {| category | where("category_id LIKE ?","#{category}") }
end
+2
source

If your intention is to make the controller thinner, you could move the logic to the model.

ProductController.rb

@products = Product.some_method(params)

Product.rb

def self.some_method(params)
  if params[:stock]
    where("stock = 0 AND active  = 1 AND title LIKE ?","%#{params[:title]}%")
  end
  if params[:category] 
    where("active = 1 AND category_id LIKE ? AND title LIKE ?", "#{params[:category]}", "%#{params[:title]}%")
  end
+1
source

Using a thin controller, the principle of a fat model.

controller :

class ProductsController < ApplicationController
  def index
    @products = Product.active(params).paginate(page: params[:page])
    @categories = Category.all
  end
end

model

class Product < ActiveRecord::Base
  belongs_to :category

  def self.active(params)
    products = where(active:1).where("title LIKE ?","%#{params[:title]}%")
    if params[:stock]
      products = products.where("stock = 0")
    end
    if params[:category]
      products = products.where("category_id LIKE ?","#{params[:category]}")
    end
  end
end
+1
source

All Articles