Topic: How to refactor this ugly code?

How to refactor this ugly code?

  def index
    @developments_all = Development.all.size
    if params[:category].blank? and params[:maker].blank? and params[:tags].blank?
      @developments_selected = Development.all.size
      @developments = Development.all.paginate(:page => params[:page], :per_page => 15)
    else
      if params[:tags].blank?
        @developments_selected = Development.category(params[:category]).maker(params[:maker]).size
        @developments = Development.category(params[:category]).maker(params[:maker]).paginate(:page => params[:page], :per_page => 15)
      else
        @developments_selected = Development.category(params[:category]).maker(params[:maker]).find_tagged_with(params[:tags].downcase, :match_all => true).size
        @developments = Development.category(params[:category]).maker(params[:maker]).find_tagged_with(params[:tags].downcase, :match_all => true).paginate(:page => params[:page], :per_page => 15)
      end
    end

    respond_to do |format|
      format.html
    end
  end

Last edited by l0pez (2009-10-05 02:18:03)

Re: How to refactor this ugly code?

It would help if you explain what you want it to do.

It looks to me like @developments_selected is just an integer representing the total number of items that have been paginated.  If so then you don't need a different variable for this, you can just call @developments.total_entries to get this number. 

Once you take out all of the "@developments_selected = ..." lines then i don't think it needs much more refactoring.  Potentially you could squeeze it onto less lines but at the expense of readability.

Last edited by Max Williams (2009-10-05 05:35:12)

###########################################
#If i've helped you then please recommend me at Working With Rails:
#http://www.workingwithrails.com/person/ … i-williams

Re: How to refactor this ugly code?

maybe it could be as clean as

@developments = Development.categorized_by(params[:category]).made_by(params[:maker]).tagged_as(params[:tags].downcase, :match_all => true).paginate(blah blah)

in your view as max says .. you can get the total count thru @development.total_entries

you need to use named_scopes in your models. eg. in your Development model  -
named_scope :categorized_by, :joins=> :development_categories, lambda{|category|
{:conditions=>['development_categories.name= ', category] } unless category.blank?
}

this is assuming that youre running on rails 2.1 + ... otherwise just find a gem called has_finder.

I think though named_scopes are generally only useful is they are used often enough.