Thanks for taking my thoughts into consideration. I'll argue my point one last time, and if you still disagree, you can do as you please. I just want Sequel to be predictable and easy to the users (especially me!).
> 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 This is what I was suggesting. > idea. It's not consistent with the rest of Sequel's behavior. > It depends on what the "rest of Sequel's behavior" is. Right now, datasets derived from a Model always yield a Model object, even a dataset eagerly loaded via #eager. The only place where this pattern breaks is with eager_graph. > 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. > I'm suggesting that each behave like all.each. All associations would be eagerly loaded before they are yeilded. Users would not be surprised at all. > 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. > Once again, the model objects returned would have cached associations, as I expect all.each to do. > Now that the documentation is explicit about using Dataset#all for > both eager and eager_graph, I don't see the problem. > The problem is that there's a pattern in Sequel where datasets derived from Models always yield Model objects to blocks passed to #each. I just checked, and even eager yields Model objects, not Hash objects. This is even true of eager_graph, as long as no arguments are passed into eager_graph. Furthermore, I would even argue that eager_graph.each currently does not behave like a graph at all, passing every row instead of building up the cached associations graph. I want to suggest that you define a method named #each_row to do what currently is being done with eager_graph.each if you want to keep the current behavior. Model.tinker.tinker.each should always yield Model objects. > 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. > Sequel 3.0 :) If you're still not convinced, I would urge you to include something in the docs for #each, explaining that #each does not guarantee what type of arguments will be passed to the block. I guess a similar explaination in #all would be useful as well: "#all always yields Model objects to the block if the dataset is a Model dataset." --David --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
