CODEWITHSUNDEEP
ruby-on-railsruby
Kyle Kaitan
Kyle Kaitan

Reputation: 1779

How can I collapse this very repetitive Ruby/Rails code?

I've got two small structural issues that I'm not sure how to handle given my relative newbie-ness with RoR.

First issue: In one of my views, I have code that looks like this:

<ul style="list-style-type: circle">
  <li><%= @apples.size %> apples</li>
  <li><%= @oranges.size %> oranges</li>
  <li><%= @bananas.size %> bananas</li>
  <li><%= @grapefruits.size %> grapefruits</li>
</ul>

Is it possible to refactor this so that I only need to iterate once over some list of different kinds of fruit, and have the appropriate <li>'s be automatically generated? Edit: I forgot to add that @apples, @oranges, etc., might be nil. Is there an idiomatic way to handle that?

Second issue: In my controller, I have code that looks like this:

@apples = Apple.find(:all)
@apples.each { |apple| apple.do_stuff(:xyz) }

@bananas = Banana.find(:all)
@bananas.each = { |banana| banana.do_stuff(:xyz) }

# ... &c

As you can see, the same operation is invoked many times in exactly the same way. Is there a way to shorten this to something like [Apple.find(:all), ...].each { |fruit| ... } and have that work instead?

Thanks very much for your help!

Upvotes: 1

Views: 265

Answers (3)

Sarah Mei
Sarah Mei

Reputation: 18484

You can actually do it pretty simply.

In your controller:

def whatever
  @fruits = {
    :apples => Apple.find(:all).each{ |a| a.do_stuff(:xyz) }, 
    :bananas => Banana.find(:all).each{ |a| a.do_stuff(:xyz) } # ... 
  }
end

In your view:

<% @fruits.each |k, v| %>
  <li><%= v.nil? ? 0 : v.size %> <%= k.to_s %></li>
<% end %>

Although you might want to consider whether do_stuff is something that could be done via a more complex finder, or by named scope.

Upvotes: 0

August Lilleaas
August Lilleaas

Reputation: 54593

I'd do this in a helper

def fruit_size(fruit)
  list = @fruits[fruit]
  return if list.empty?

  content_tag(:li, "#{list.size} #{fruit}")
end

And this in the view:

<% ["apples", "oranges", "bananas", .....].each do |fruit| %>
  <%= fruit_size(fruit)
<% end %>

In your controller:

@fruits = {}
["apples", "oranges", "bananas", ......].each do |fruit|
  @fruits[fruit] = fruit.classify.constantize.find(:all).each {|record|
    record.whatever_here
  end
end

It makes sense to store all the items in a hash, @fruits, so that you don't have to use instance_variable_get and stuff.

Perhaps you also want to define that array somewhere, so that you don't have to repeat it in the controller and in the view. Let's pretend that you have a fruit model.

class Fruit < ActiveRecord::Base
  FRUITS = ["apples", "oranges", "bananas", ....]
end

Then, use Fruit::FRUITS in the view and controller.

Upvotes: 5

Brian
Brian

Reputation: 4930

For the first part:

    @li = ''
    [@apples, @oranges, @bananas, @grapefruit].each{|fruit| 
       @li << "<li>#{fruit.size}</li>"}

    <ul style="list-style-type: circle">
    <%=@li%>
    </ul>

Upvotes: 0

Related Questions