CODEWITHSUNDEEP
ruby-on-railsruby
BushyMark
BushyMark

Reputation: 3385

What is wrong with code like User.find(:all).each?

I just read this requirement on a job listing:

Aware of the pitfalls of code like: User.find(:all).each

I knew instantly that I was unqualified for the job because for the life of me, I don't understand what the problem is. Is it...

Upvotes: 3

Views: 1788

Answers (5)

Jure Triglav
Jure Triglav

Reputation: 1912

It's a pitfall, I'm not a native English speaker, but my understanding is that it's not an immediate drawback, but it's a hole in the ground you should be wary of. Doing it like this is future proof:

User.find_in_batches do |group|
  group.each do |user|
    user.do_something
  end
end

Even if you grow from 1 user to 10 million in a week.

Upvotes: 1

Simon Woodside
Simon Woodside

Reputation: 7304

Well, if you want to sound smart, you answer is: There is no pitfall to that question, assuming that you want to modify or display values from each and every one of the user objects. :-)

Upvotes: 0

Ryan Bigg
Ryan Bigg

Reputation: 107728

Doing User.all will load in all the user records. If you have 3 million records, it will load in all 3 million objects. This is why it is a bad idea. It's best to filter down your SQL using methods like pagination or conditions to return the smallest subset needed to "get the job done"

Upvotes: 5

Kevin Peterson
Kevin Peterson

Reputation: 7297

I think the "pitfall" they are looking for is that when someone writes User.all.each, it usually looks like this:

User.all.each do |u|
    next if !u.is_active
    ...
end

meaning that the filtering is happening in Ruby, after having loaded the entire contents of each of those objects from the DB, when the filtering could have been done much more efficiently by expressing the desired property as part of the query.

Upvotes: 7

kch
kch

Reputation: 79542

Is it wordy? You can do the same with User.all.each ? (-1 word! w00t!)

We do appreciate brevity in the land of ruby. I, for one, vote for implementing Model.each, now that you made me consider it.

Is it simply poorly worded? Should it be prefaced with "The users table happens to have 3 million rows"

I believe this is the most reasonable answer. You may be loading a lot of records into memory.

I'd say the problem is not so much that the users table happens to have 3m records, but that it may come to have them within a reasonable timeframe.

Upvotes: 2

Related Questions