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

Question

I just read this as a requirement on a job listing:

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

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

Is it design related? Store the database request in a variable and THEN iterate over it?

Is it dangerous?

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

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

Any clarification is much appreciated!

Answer 1

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.

Answer 2

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.

Answer 3

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"

Answer 4

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. :-)