On Mar 18, 1:33 pm, David Lee <[email protected]> wrote: > On Mar 18, 11:59 am, Jeremy Evans <[email protected]> wrote: > > On Mar 18, 11:47 am, David Lee <[email protected]> wrote: > > > Shouldn't #each implicitly do what #all does (set up all associations) > > > before returning? I really think all.each and each should behave > > > exactly the same. > > > No, it shouldn't. each has always yielded each record to its block as > > soon as it is yielded by the adapter, and all has always consumed all > > records before yielding/returning. Making each act like all would > > cause problems. > > Of course, when the block is yielded would be different. By "set up > all associations," I meant, "wrap the returned object in an Item > object and link all associated objects." I'm sorry it wasn't clear.
You can't wrap the hash of hashes in an Item object, or at least it would require throwing away the information for all of the other tables. You can't link all associated objects, because all associated objects wouldn't be available, only the objects for the current row would be. If you are implying that #each should deduce when it is being used by eager_graph and basically act like an alias to #all, I don't like that idea. It's not consistent with the rest of Sequel's behavior. You should think of eager_graphed datasets as regular graphed datasets with some special sauce sprinkled in all to take each row's hash and transform it into a object graph. Dataset#each for a graphed dataset always yields a hash of hashes. > Sequel should adhere to POLS, but I was surprised that all.each and > each yielded different objects. I was especially surprised because > Item.each, Item.filter.each, Item.dataset.each, Item.join(...).each, > and even Item.eager_graph().each all yield Item objects, but > Item.eager_graph(:user).each yields Hash objects. I can see what you are saying, but your suggestion would do more harm than good. If you are using eager_graph, you should be calling Dataset#all unless you know what you are doing (i.e. you want a hash with model object values yielded for each row). Changing Dataset#each for an eager_graphed dataset to return model objects would break the POLS because people would think the associations are eagerly loaded in the objects yielded, when they are not. > Item.eager_graph(...).each should yield Item objects for consistency's > sake and not doing so should be considered a bug. No. Don't let the hobgoblin get you. > If we have to trade off between timing of the yields and the > properness of the parameters passed to the yields, we should prefer > the properness of the parameters. I think the current situation is better. If a person thinks they can use Dataset#each on an eager_graphed dataset, they will figure out pretty soon that it doesn't work that way and they need to use Dataset#all. This is much preferable to the alternative, which would be people coming here complaining that their eager_graphed datasets are yielding model objects without any cached associations, as if eager_graph wasn't even called. Now that the documentation is explicit about using Dataset#all for both eager and eager_graph, I don't see the problem. Not to mention that I'm wouldn't be surprised if some people are relying on the fact that Dataset#each for an eager_graphed dataset yields a hash with model object values (it always has), so even if I thought this idea had merit, I'd have to consider the backwards compatibility issues. Jeremy --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "sequel-talk" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/sequel-talk?hl=en -~----------~----~----~----~------~----~------~--~---
