Adding to an array in Ruby on Rails

Question

I'm currently writing a ruby function that gets a table from a database and then based on the data creates an array. It's written like this:

  def listSome
    @foos = Array.new
    FooTable.find(:all) do |foo|
      @foos << foo if foo.name == "Bar"
    end
  end

The problem I'm having is that only the first element of the query is getting added to the array. I've checked that FooTable.find(:all) returns what I think it should in the console and also that it's ok to loop off of it's results (I printed the results on every loop, it found what it was looking for). I suspect, however, there is something about concatenation to arrays/collections that I don't understand. Why am I only getting the first result of the query added into my array? Thanks.

Samuel · Accepted Answer · 2011-01-28 17:27:09Z

8

You are providing a block to the find method which isn't going to run it against each element of the array it returns. Provide your block to the each method returned by find.

FooTable.find(:all).each { |foo| ... }

Also, assuming this is actual code and not an example, there is a really bad way of getting foos with a specific name.

rails 2

@foos = FooTable.find(:all, :conditions => ['name = ?', 'Bar'])

rails 3

@foos = FooTable.where('name = ?', 'Bar')

answered Jan 28, 2011 at 17:27

Samuel

38.5k11 gold badges88 silver badges87 bronze badges

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

3 Comments

jdl Over a year ago

+1 I deleted a similar answer, but didn't mention the inefficiency of his find. I like this this one better.

keybored Over a year ago

Jeez you're right I see it now. Also, would this be any more or less a valid/better/worse approach? @foos = FooTable.find_all_by_name("Bar")?

Jacob Mattison Over a year ago

Using find_all_by_name would be a perfectly good approach; that's a dynamic helper that would end up being exactly the same as doing the find with a condition.

tokland · Accepted Answer · 2011-01-28 17:35:01Z

0

You forgot the each: FooTable.find(:all).each do |foo|.

But I'd make some more comments, you should use more map/select/reject/inject and less each:

def listSome
  @foos = FooTable.find(:all).select do |foo|
    foo.name == "Bar"
  end
end

But whenever possible, use more SQL and less Ruby:

def listSome
  @foos = FooTable.where(:name => "Bar")
end

edited Jan 28, 2011 at 17:35

answered Jan 28, 2011 at 17:27

tokland

68.2k13 gold badges151 silver badges174 bronze badges

2 Comments

Samuel Over a year ago

Why use map and compact instead of just a select?

tokland Over a year ago

@Samuel, indeed, I wanted to show map, but here select is better.

Collectives™ on Stack Overflow

Adding to an array in Ruby on Rails

2 Answers 2

3 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related