Rails - moving controller logic into a model

Question

I'm looking to clean up my controller, as it seems heavy and redundant. Any help on how I'd go about moving this type of logic into my model would be appreciated. Thanks for any help on this - the code below is my for index action:

case params[:find_by]
  when 'topic'
    nuggets = Nugget.where(['topic = ?', params[:topic_name]])
    @nuggets = nuggets.paginate(:page => params[:page],:per_page => 15)
    @title = nuggets.first.topic
  when 'audience'
    nuggets = Nugget.where(['audience = ?', params[:audience_name]])
    @nuggets = nuggets.paginate(:page => params[:page], :per_page => 15)
    @title = nuggets.first.audience
  else
    @nuggets = Nugget.paginate(:page => params[:page], :per_page => 15)
end

Looks like something like MetaSearch, MetaWhere, or Ransack could help you here. — Daniel Rikowski
– Daniel Rikowski, Commented Nov 6, 2011 at 19:47

Dave Newton · Accepted Answer · 2011-11-06 18:51:23Z

1

I'm not entirely sure I would move it into the model. I'd probably just move it into a private utility method in the controller.

case params[:find_by]
  when 'topic'
    nuggets = Nugget.find_by_topic(params[:topic_name])
    @title = nuggets.first.topic
  when 'audience'
    nuggets = Nugget.find_by_audience(params[:audience_name])
    @title = nuggets.first.audience
  else
    nuggets = Nugget.all
end

@nuggets = Nugget.paginate(:page => params[:page], :per_page => 15)

Another option would be to create routes for the different finds; whether or not it's worth it, meh. You could move the find_by logic into the model, or use a send to slightly DRY up the topic/audience difference, but again, that seems more trouble than it's worth.

I'll be interested to see what more Rails-y people think about the question, though.

answered Nov 6, 2011 at 18:51

Dave Newton

161k27 gold badges264 silver badges311 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Gavin Over a year ago

Thanks Dave - based on what I need to do, I'm going to create separate routes. I played around with it for a bit and found that to be my best option - thanks for the input!

Dave Newton Over a year ago

@Gavin No problem--sometimes it's the cleanest, easiest-to-maintain thing to do, for better or worse.

Hardik Joshi · Accepted Answer · 2011-11-06 19:26:59Z

1

Add this to your model

def self.topic(topic_name)
    where(:topic => topic_name)
end

def self.audience(audience_name)
    where(:audience => audience_name)
end

And in the controller replace with this

def index
    if params[:find_by]
        nuggets = Nugget.send(params[:find_by].to_sym)(params[:name])
        @nuggets = nuggets.paginate(:page => params[:page], :per_page => 15)
    else
        @nuggets = Nugget.paginate(:page => params[:page], :per_page => 15)
    end
    @title = @nuggets.first.send(params[:find_by].to_sym)
end

You will just need to use same parameter key for audience and topic

answered Nov 6, 2011 at 19:26

Hardik Joshi

8392 gold badges8 silver badges15 bronze badges

1 Comment

Dave Newton Over a year ago

I find that less obvious and more difficult to read than the naive solution (which is why I only mentioned it in passing in my answer).

Collectives™ on Stack Overflow

Rails - moving controller logic into a model

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related