9d4eaaf6c763c9fc01a9356ef58dbe72

Need some help refactoring this if/else block that builds the conditions for a find query.

if params[:status] && params[:carrier]
  conditions = ["actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery AND status_id = ? AND carrier_id = ?", status.id, carrier.id]
elsif params[:status]
  conditions = ["actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery AND status_id = ?", status.id]
elsif params[:carrier]
  conditions = ["actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery AND carrier_id = ?", carrier.id]
else
  conditions = ["actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery"]
end

@packages = Package.find(:all, :conditions => conditions)

Refactorings

No refactoring yet !

A8d3f35baafdaea851914b17dae9e1fc

Adam

October 29, 2010, October 29, 2010 19:23, permalink

4 ratings. Login to rate! 
class Package < ActiveRecord::Base
  scope :delivered_after_scheduled, where('actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery')
  scope :carrier, lambda { |carrier| where(:carrier_id => carrier.id) }
  scope :status, lambda { |status| where(:status_id => status.id) }
end

class PackagesController < ApplicationController
  def index
    scope = Package
    scope = scope.carrier(carrier) if carrier
    scope = scope.status(status) if status
    
    @packages = scope.delivered_after_scheduled
  end
end
D41d8cd98f00b204e9800998ecf8427e

Case Nelson

November 6, 2010, November 06, 2010 16:29, permalink

No rating. Login to rate!

I'm new to rails and I found the scope variable in the controller confusing as it clashes with the method name on the model. It looks like this is why .scoped exists

class Package < ActiveRecord::Base
  scope :delivered_after_scheduled, where('actual_delivery IS NOT NULL AND actual_delivery > scheduled_delivery')
  scope :carrier, lambda { |carrier| where(:carrier_id => carrier.id) }
  scope :status, lambda { |status| where(:status_id => status.id) }
end

class PackagesController < ApplicationController
  def index
    packages = Package.scoped
    packages = packages.carrier(carrier) if carrier
    packages = packages.status(status) if status
    
    @packages = packages.delivered_after_scheduled
  end
end

Your refactoring





Format Copy from initial code

or Cancel