Re: Some new iterator methods?

[prev] [thread] [next] [Date index for 2004/11/10]

On Tue, 26 Oct 2004, Tony Bowden wrote:

> On Tue, Oct 26, 2004 at 01:13:41PM -0400, John Day wrote:
> > wish I had a couple of new iterator methods, particularly:
> > ->previous
> > ->last
> >
>
> These shouldn't be too difficult to implement. There are some issues due
> to the possibility of mapping methods expanding into multiple objects,
> but if you implement each of these in terms of next() (or factor out the
> part of next that selects the current location from the part that gets
> the item there) it should reduce most breakage. (If someone really wants
> to iterate through lists that may change in size then they can send the
> patch to allow it!)

Just for fun, I decided to take a shot at this.  Because of the way
next() currently works, previous() was harder (for me) to get
working the way I imagined it should work.  But it seems to be
working correctly now.

Here are my patches for Class::DBI::Iterator and the 21-iterator.t
test (both against 0.96):

--------------------BEGIN Iterator.pm PATCH--------------------
--- lib/Class/DBI/Iterator.pm.orig	Sun Apr 25 10:33:36 2004
+++ lib/Class/DBI/Iterator.pm	Thu Nov  4 07:56:12 2004
@@ -13,6 +13,9 @@
 	my $first_result = $it->first;
 	while ($it->next) { ... }

+	my $last_result = $it->last;
+	while ($it->previous) { ... }
+
 	my @slice = $it->slice(10,19);
 	my $slice = $it->slice(10,19);

@@ -50,6 +53,7 @@
 		_data   => $data,
 		_mapper => [@mapper],
 		_place  => 0,
+		_is_reset => 1
 		},
 		ref $me || $me;
 }
@@ -71,13 +75,26 @@

 sub next {
 	my $self = shift;
-	my $use  = $self->{_data}->[ $self->{_place}++ ] or return;
-	my @obj  = ($self->class->construct($use));
-	foreach my $meth ($self->mapper) {
-		@obj = map $_->$meth(), @obj;
+
+	return if $self->{_place} >= $self->count;
+
+	if ( $self->{_is_reset} ) {
+		$self->{_is_reset} = 0;
+		return $self->_get_item;
+	}
+	else {
+		$self->{_place}++;
+		return $self->_get_item;
 	}
-	warn "Discarding extra inflated objects" if @obj > 1;
-	return $obj[0];
+}
+
+sub previous {
+	my $self = shift;
+
+	return if $self->{_place} < 0;
+
+	$self->{_place}--;
+	return $self->_get_item;
 }

 sub first {
@@ -86,12 +103,32 @@
 	return $self->next;
 }

+sub last {
+	my $self = shift;
+	$self->{_place} = $self->count - 1;
+	return $self->_get_item;
+}
+
+sub _get_item {
+	my $self = shift;
+	return if $self->{_place} < 0 || $self->{_place} >= $self->count;
+	my $use  = $self->{_data}->[ $self->{_place} ] or return;
+
+	my @obj  = ($self->class->construct($use));
+	foreach my $meth ($self->mapper) {
+		@obj = map $_->$meth(), @obj;
+	}
+	warn "Discarding extra inflated objects" if @obj > 1;
+	return $obj[0];
+}
+
 sub slice {
 	my ($self, $start, $end) = @_;
 	$end ||= $start;
 	$self->{_place} = $start;
+	$self->{_is_reset} = 1;
 	my @return;
-	while ($self->{_place} <= $end) {
+	while ($self->{_place} < $end) {
 		push @return, $self->next || last;
 	}
 	return @return if wantarray;
@@ -111,6 +148,10 @@
 	1;
 }

-sub reset { shift->{_place} = 0 }
+sub reset {
+	my $self = shift;
+	$self->{_place} = 0;
+	$self->{_is_reset} = 1;
+}

 1;
---------------------END Iterator.pm PATCH---------------------

-------------------BEGIN 21-iterator.t PATCH-------------------
--- t/21-iterator.t.orig	Thu Nov  4 07:59:12 2004
+++ t/21-iterator.t	Thu Nov  4 07:30:05 2004
@@ -3,7 +3,7 @@

 BEGIN {
 	eval "use DBD::SQLite";
-	plan $@ ? (skip_all => 'needs DBD::SQLite for testing') : (tests => 33);
+	plan $@ ? (skip_all => 'needs DBD::SQLite for testing') : (tests => 43);
 }

 INIT {
@@ -34,6 +34,14 @@
 		my $from2 = $it2->next;
 		is $from1->id, $from2->id, "Both iterators get $from1";
 	}
+
+	$it1->last;
+	$it2->last;
+
+	while (my $from1 = $it1->previous) {
+		my $from2 = $it2->previous;
+		is $from1->id, $from2->id, "Both iterators get $from1";
+	}
 }

 {
@@ -45,6 +53,10 @@
 	$it->reset;
 	is $it->next->title, "Film 1", "Reset brings us to film 1 again";
 	is $it->next->title, "Film 2", "And 2 is still next";
+	is $it->previous->title, 'Film 1', '1 is the previous film';
+
+	is $it->last->title, 'Film 6', 'Film 6 last';
+	is $it->previous->title, 'Film 5', 'Film 5 previous';
 }


@@ -68,6 +80,8 @@
 	$slice->reset;
 	is $slice->next->title, "Film 3", "Reset brings us to film 3 again";
 	is $slice->next->title, "Film 4", "And 4 is still next";
+	is $slice->last->title, 'Film 5', 'Film 5 is last';
+	is $slice->previous->title, 'Film 4', 'And 4 is previous to that';

 	# check if the original iterator still works
 	is $it->count, 6, "back to the original iterator, is of right size";
--------------------END 21-iterator.t PATCH--------------------

Like I said, this all works for me.  I'm curious to know how well it
works for others, and also whether previous() is working the way
folks would expect it to.

Take care,

Dave

/L\_/E\_/A\_/R\_/N\_/T\_/E\_/A\_/C\_/H\_/L\_/E\_/A\_/R\_/N\
Dave Cash                              Power to the People!
Frolicking in Fields of Garlic               Right On-Line!
dave@xxxxx.xxx                                  Dig it all.