[Dbix-class] Improved resultset iterators

[Toby Corkindale]

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