CODEWITHSUNDEEP
ruby-on-railsruby
Cameron
Cameron

Reputation: 28803

Get data from Helper or use Model in Rails

In my Rails app when creating a new Article I need a list of Users and Categories for some drop-downs so that I can choose both a category and an author for the Article.

Currently I do this in the controller:

def new
  @article = Article.new
  @categories = Category.order('title asc').collect { |t| [t.title, t.id] }
  @users = User.order('email asc').collect { |t| [t.email, t.id] }
end

And then in the view:

<%= f.select :category_id, options_for_select(@categories), :prompt => '-- Please Select --', :required => true %>

But according to RubyDocs this is bad practice, and it's not very DRY as I then have to do this for the edit method too. To prevent this, I have two possible alternatives I can think of:

1.) Use a helper like this:

def users_for_select
  User.order('email asc').collect { |t| [t.email, t.id] }
end

def categories_for_select
  Category.order('title asc').collect { |t| [t.title, t.id] }
end

And then in the view:

<%= f.select :category_id, options_for_select(categories_for_select), :prompt => '-- Please Select --', :required => true %>

2.) Move it to a Model:

def self.categories_for_select
  Category.order('title asc').collect { |t| [t.title, t.id] }
end

def self.users_for_select
  User.order('email asc').collect { |t| [t.email, t.id] }
end

And then in the controller do this:

def new
  @article = Article.new
  @categories = Category.categories_for_select
  @users = User.users_for_select
end

Option 1 feels cleaner as it removes the code from the controller, but I was under the impression that option 2 would be better as it uses a Model for data (as intended) and the controller is still sending the data (again as intended) but more DRY.

I feel their is a sometimes some overlap between Helpers and Models for getting data.

Upvotes: 1

Views: 567

Answers (3)

Cent
Cent

Reputation: 881

I'll personally go with Option 1.

Sure, you could put that in your model. You'll find sooner than later that that would be a great way to bloat up the model. Then you might thinking about using concerns to hide the bloat. And the messy trend continues.

Here's why I think option 1 is better. Even though you aren't creating a separate class to handle formatting, you're still abstracting functionality into a smaller segment which is easier to scale. Plus of course, composition beats inheritance.

This awesome post by Bryany gives great options on refractoring fat models.

As @damien already pointed out in his answers you'll want to use ActiveRecord's pluck instead of ruby's collect. Pluck queries the db so that it returns only the objects you need.

Upvotes: 1

spickermann
spickermann

Reputation: 106882

I would use a helper method for this:

# in a helper
def category_options_for_select
  options_for_select(Category.order(:title).pluck(:title, :id))
end

# in the view
<%= f.select :category_id, category_options_for_select, prompt: '-- Please Select --', required: true %>

Upvotes: 1

user419017
user419017

Reputation:

I would go with (1) your helper method for now. It's simple and straightforward. As I said in the comments, you could use a decorator around your model (using draper for e.g.) to add what I'd consider quite view-specific logic if you are tempted towards option (2).

One note on your helper methods - use pluck instead of collect so you don't select columns or instantiate a bunch of objects you don't need.

Also, order defaults to asc, so you can shorten the whole thing to:

def users_for_select
  User.order(:email).pluck(:email, :id)
end

Upvotes: 2

Related Questions