Filtering users by criteria in Ruby on Rails I am a fairly new (RubyonRails) developer. I desire to improve my coding skills so I used climate to do some review on my code. It gave me a flag for this method that its complex. Is it characterised as complex because of having several "actions/tasks" in a single method?

Will it be better if I extract some code segments to a different method?

Is there something else I am not seeing?

  def search
    filter_mapping = {"cost" =&gt; "rcost", "quality" =&gt; "rquality", "time" =&gt; "rtime", "experience" =&gt; "rexperience", "communication" =&gt; "rcommunication"}
    @filters = params[:filter].split(",") rescue []
    @sort = params[:sort]

    @user_type = params[:s_user_type]
    @skills = params[:s_skills]
    @location = params[:location]
    @max_rate = params[:max_rate]
    @availability = params[:availability]

    @users = User.scoped.joins(:user_skills)
    @users = @users.where('user_type = ?', @user_type) if @user_type.present?
    @users = @users.where('user_skills.skill_id in (?)', @skills.map(&amp;:to_i)) if @skills.present? &amp;&amp; @skills.size &gt; 0
    @users = @users.where('availability = ?', @availability) if @availability.present?
    @users = @users.where('location_country = ?', @location) if @location.present?
    @users = @users.where('rate &lt; ?', @max_rate.to_i) if @max_rate.present?
    @users = @users.page(params[:page]).per(PER_PAGE)

    @filters.each do |f|
      if filter_mapping.keys.include?(f)
        @users = @users.order("#{filter_mapping[f]} desc")
      end
    end

    @users = @users.order('id asc') if @filters.empty?
    @advanced_link = @location.blank? &amp;&amp; @max_rate.blank? &amp;&amp; @availability.blank?
    render :index
  end


Update

I figured out that I can extract the scopes into a method like that:

  def get_users_where_scopes
    @users = User.scoped.joins(:user_skills)
    @users = @users.where('user_type = ?', @user_type) if @user_type.present?
    @users = @users.where('user_skills.skill_id in (?)', @skills.map(&amp;:to_i)) if @skills.present? &amp;&amp; @skills.size &gt; 0
    @users = @users.where('availability = ?', @availability) if @availability.present?
    @users = @users.where('location_country = ?', @location) if @location.present?
    @users = @users.where('rate &lt; ?', @max_rate.to_i) if @max_rate.present?
    @users = @users.page(params[:page]).per(PER_PAGE) 
  end


and then call it with @users = get_users_where_scopes(). But now the complexity of this method seems wrong to me.
I'd say to first make a service object to keep the controller lean and clean, and to give yourself a place to put all the logic without fear of polluting the controller. Plus: It's reusable!

# app/services/user_search.rb

class UserSearch
  ORDER_MAPPING = {
    "cost"          =&gt; "rcost",
    "quality"       =&gt; "rquality",
    "time"          =&gt; "rtime",
    "experience"    =&gt; "rexperience",
    "communication" =&gt; "rcommunication"
  }.freeze

  def initialize(params)
    @params = params
  end

  def results
    @results ||= begin
      records = User.scoped.joins(:user_skills)
      records = scope(records)
      records = order(records)
    end
  end

  private

  def param(key)
    @params[key] if @params[key].present?
  end

  def scope(scoped)
    scoped = add_scope(scoped, 'user_type = ?', param(:user_type))
    scoped = add_scope(scoped, 'user_skills.skill_id in (?)', skill_ids)
    scoped = add_scope(scoped, 'availability = ?', param(:availability))
    scoped = add_scope(scoped, 'location_country = ?', param(:location))
    scoped = add_scope(scoped, 'rate &lt; ?', max_rate)
  end

  def add_scope(scope, sql, *params)
    scope.where(sql, *params) if params.all?(&amp;:present?)
    scope
  end

  def order(scope)
    terms = sanitized_order_terms || default_order_terms
    terms.each { |term| scope.order(term) }
    scope
  end

  def sanitized_order_terms
    terms = param(:filter).try(:split, ",")
    terms = terms.map { |term| ORDER_MAPPING[term] }
    terms = terms.compact
    terms if terms.any?
  end

  def default_order_terms
    ["id asc"]
  end

  def skill_ids
    param(:s_skills).try(:map, &amp;:to_i)
  end

  def max_rate
    param(:max_rate).try(:to_i)
  end
end


I've intentionally kept the pagination in the controller, as it's pretty independent of the scoping and ordering. However, it'd be simple to add as arguments to the #results method

In your controller:

def search
  @users = UserSearch.new(params).results.page(params[:page]).per(PER_PAGE)

  advanced_params = %w(location max_rate availability).map { |p| params[p] }
  @advanced_link = advanced_params.all?(&amp;:blank)

  render :index
end


I'd probably pick a more direct way of determining the @advanced_link, such as sending an advanced parameter along, and simply looking at that instead of the implicit state you have now.

I have no idea what Code Climate thinks of the code above, but I imagine it'll be happier.