[Dbix-class] Improved resultset iterators

[Toby Corkindale]

Hi,
I posted this to the list in May, just looking for confirmation that the 
API I had in mind was correct, before implementing it.
(Since my first, implemented suggestion had been shot down, and I'd 
rather nail the spec down before coding it again.)

No response at the time, but I thought I'd try again now.
Cheers! See below for background topic and then my comments.

On 07/05/10 06:31, Matt S Trout wrote:
> On Tue, May 04, 2010 at 01:08:52PM +1000, Toby Corkindale wrote:
>> On 22/04/10 19:00, Peter Rabbitson wrote:
>>> Toby Corkindale wrote:
>>>> Hey all,
>>>> Wouldn't it be nice if the ResultSet iterators were a bit more advanced?
>>>>
>>>> I would love it if the iterator built into DBIx::Class supported some
>>>> more functional programming style methods.
>>>>
[snip examples]
>>>
>>> The implicit iterators built into DBIC resultsets were a blatant design
>>> mistake, and therefore they will not be extended any longer (at least not
>>> in core). You are however welcome to discuss a design for a ResultSet
>>> component, or even contribute to the DBIx::Class::Helpers family of
>>> modules.
>>
>> I attach a ResultSet Component that adds the features I was discussing.
>>
>> Would this be appropriate to release as a mini CPAN module or would you
>> like to incorporate it into something else?
>
> I would like you to never release that, and erase all copies from your hard
> disk.
>
> As noted, they are a design mistake. One you are perpetuating.
>
> If you want to do something useful, write a resultset component that
> extracts the iterator work out into a separate object and proxies next,
> first and reset to a built-in iterator.

Oh, sorry, I thought I was following Peter's suggestion about making a
Helper component.

OK, let's talk about your suggestion a little more. How does this sound:

ResultSet gains an iter/iterator method, which returns a new iterator.

ResultSet gains a, uh, _current_iterator() object property, used for
keeping the next/first/reset methods backwards compatible.

Calling ->next or ->first fetches $self->_current_iterator and then
calls next/first upon it. (If $self->current_iterator isn't set, it
assigns it the value of $self->iter, then calls next/first)

Calling ->reset reassigns $self->iter to $self->_current_iterator.

so, uh:
sub reset { $_[0]->_current_iter($_[0]->iter) }
sub next  {
   my $self = shift;
   unless ($self->_current_iter) { $self->_current_iter($self->iter) };
   return $self->_current_iter->next;
} # ditto for first()
sub iter {
   return DBIC::RS::Iterator->new(resultset => shift);
}


Regarding the Iterator itself..
Do you think it should setup a ResultSet->cursor() in order to handle
the iterations, or should it effectively just call ResultSet->all and
store that in a local array?


> Then you cold implement things like foreach as
>
> $self->iter->each(...
>
> or similar. Which would be far cleaner.

>
>> Also, I wondered at which point it is best to call $resultset->reset..
>> At the start, or end, or both, of all the methods?
>
> Any design that requires ->reset to be called is perpetuating the same
> mistake as perl's each() builtin makes and should be taken out and shot.

Hey, I'm just proxying a lot of calls to ->next in the code I
submitted.. not my fault the underlying system required ->reset, is it? :)


Cheers,
Toby