Re: Bug: DT::Event::Recurrence Modifies Params
Yitzchak wrote: YMMV, but I find something like this more clear: @args = sort { ($a 0) = ($b 0) || $a = $b } @args; (literally, sort first by positive/negative, then by value). Very nice - thanks! Actually, I'm planning to rewrite this part of the program, because it can't handle overflow cases like this one: monthly ( days = [ 30, -20 ] ) - Flavio S. Glock
Re: Bug: DT::Event::Recurrence Modifies Params
Daisuke wrote: +sub _sort_positive_first +{ +my @sorted = sort { $a = $b } @_; +# put positive values first +my @ret = grep { $_ = 0 } @sorted; +push @ret, $_ for grep { $_ 0 } @sorted; + +return @ret; +} I rewrote the sort: @args = sort { $a 0 ? ( $b 0 ? $a = $b : 1 ) : ( $b 0 ? -1 : $a = $b ) } @args; DateTime::Event::Recurrence was uploaded to CPAN as version 0.15. I added a test for modified parameter. Thank you! - Flavio
Re: Bug: DT::Event::Recurrence Modifies Params
On Wed, Jan 19, 2005 at 04:59:33PM +, [EMAIL PROTECTED] wrote: Daisuke wrote: +sub _sort_positive_first +{ +my @sorted = sort { $a = $b } @_; +# put positive values first +my @ret = grep { $_ = 0 } @sorted; +push @ret, $_ for grep { $_ 0 } @sorted; + +return @ret; +} I rewrote the sort: @args = sort { $a 0 ? ( $b 0 ? $a = $b : 1 ) : ( $b 0 ? -1 : $a = $b ) } @args; DateTime::Event::Recurrence was uploaded to CPAN as version 0.15. I added a test for modified parameter. YMMV, but I find something like this more clear: @args = sort { ($a 0) = ($b 0) || $a = $b } @args; (literally, sort first by positive/negative, then by value).
Re: Bug: DT::Event::Recurrence Modifies Params
On Sat, Jan 08, 2005 at 09:56:38AM +0900, Daisuke Maki wrote: +sub _sort_positive_first +{ +my @sorted = sort { $a = $b } @_; +# put positive values first +my @ret = grep { $_ = 0 } @sorted; +push @ret, $_ for grep { $_ 0 } @sorted; push takes a list; make that push @ret, grep { $_ 0 } @sorted; + +return @ret; +}
Re: Bug: DT::Event::Recurrence Modifies Params
I noticed a little bug today. DateTime::Event::Recurrence modifies the parameters that are passed to it. I suspect these lines.. Index: lib/DateTime/Event/Recurrence.pm === RCS file: /cvsroot/perl-date-time/modules/DateTime-Event-Recurrence/lib/DateTime/Event/Recurrence.pm,v retrieving revision 1.85 diff -d -u -r1.85 Recurrence.pm --- lib/DateTime/Event/Recurrence.pm22 Sep 2004 16:19:23 - 1.85 +++ lib/DateTime/Event/Recurrence.pm7 Jan 2005 09:58:58 - @@ -684,11 +684,11 @@ if ( $unit eq 'days' ) { # map rfc2445 weekdays to numbers -@{$args{$unit}} = map { +$args{$unit} = [ map { $_ =~ /[a-z]/ ? $_ = $weekdays{$_} : $_ -} @{$args{$unit}}; +} @{$args{$unit}} ]; } @{$args{$unit}} = sort { $a = $b } @{$args{$unit}}; # put positive values first However, I don't think this solves the whole problem. It looks like there are a lot of @{ $args { $foo } } = @blah_blah; type of constructs, and that's where this is all coming from. If no one objects, I don't mind doing a sweep change and remove all such occurences. It's a three day weekend over in this corner of Asia. --d
Re: Bug: DT::Event::Recurrence Modifies Params
On Fri, 7 Jan 2005, Daisuke Maki wrote: I suspect these lines.. Index: lib/DateTime/Event/Recurrence.pm === RCS file: /cvsroot/perl-date-time/modules/DateTime-Event-Recurrence/lib/DateTime/Event/Recurrence.pm,v retrieving revision 1.85 diff -d -u -r1.85 Recurrence.pm --- lib/DateTime/Event/Recurrence.pm22 Sep 2004 16:19:23 - 1.85 +++ lib/DateTime/Event/Recurrence.pm7 Jan 2005 09:58:58 - @@ -684,11 +684,11 @@ if ( $unit eq 'days' ) { # map rfc2445 weekdays to numbers -@{$args{$unit}} = map { +$args{$unit} = [ map { $_ =~ /[a-z]/ ? $_ = $weekdays{$_} : $_ -} @{$args{$unit}}; +} @{$args{$unit}} ]; } @{$args{$unit}} = sort { $a = $b } @{$args{$unit}}; # put positive values first However, I don't think this solves the whole problem. It looks like there are a lot of @{ $args { $foo } } = @blah_blah; type of constructs, and that's where this is all coming from. If no one objects, I don't mind doing a sweep change and remove all such occurences. It's a three day weekend over in this corner of Asia. That seems like a good idea. Messing with your callers' arguments is generally bad form. -dave /*=== VegGuide.Org Your guide to all that's veg. ===*/
Re: Bug: DT::Event::Recurrence Modifies Params
can Flavio/Dave/somebody review this code? I basically opted to copy over $args{$unit} to a different array before doing any processing, and refactored one bit of code. All existing tests passed. --d Index: lib/DateTime/Event/Recurrence.pm === RCS file: /cvsroot/perl-date-time/modules/DateTime-Event-Recurrence/lib/DateTime/Event/Recurrence.pm,v retrieving revision 1.85 diff -d -u -r1.85 Recurrence.pm --- lib/DateTime/Event/Recurrence.pm22 Sep 2004 16:19:23 - 1.85 +++ lib/DateTime/Event/Recurrence.pm8 Jan 2005 00:54:16 - @@ -619,6 +619,15 @@ return $set; } +sub _sort_positive_first +{ +my @sorted = sort { $a = $b } @_; +# put positive values first +my @ret = grep { $_ = 0 } @sorted; +push @ret, $_ for grep { $_ 0 } @sorted; + +return @ret; +} # method( hours = 10 ) # method( hours = 10, minutes = 30 ) @@ -676,8 +685,13 @@ next unless exists $args{$unit}; -$args{$unit} = [ $args{$unit} ] -unless ref( $args{$unit} ) eq 'ARRAY'; +# copy the argument values before proceeding, so that we +# don't accidentally modify it. +if (ref($args{$unit}) eq 'ARRAY') { +$args{$unit} = [ @{$args{$unit}} ]; +} else { +$args{$unit} = [ $args{$unit} ]; +} # TODO: sort _after_ normalization @@ -690,13 +704,7 @@ $_ } @{$args{$unit}}; } -@{$args{$unit}} = sort { $a = $b } @{$args{$unit}}; -# put positive values first -# warn Arguments: $unit = @{$args{$unit}}; -my @tmp = grep { $_ = 0 } @{$args{$unit}}; -push @tmp, $_ for grep { $_ 0 } @{$args{$unit}}; -# print STDERR $unit = @tmp\n; -@{$args{$unit}} = @tmp; +$args{$unit} = [ _sort_positive_first(@{$args{$unit}}) ]; $ical_string .= uc( ';' . $ical_name{$unit} . '=' . join(,, @{$args{$unit}} ) ) unless $unit eq 'nanoseconds'; @@ -755,14 +763,7 @@ } # redo argument sort - -@{$args{$unit}} = sort { $a = $b } @{$args{$unit}}; -# put positive values first -my @tmp = grep { $_ = 0 } @{$args{$unit}}; -push @tmp, $_ for grep { $_ 0 } @{$args{$unit}}; -# print STDERR $unit = @tmp\n; -@{$args{$unit}} = @tmp; - +$args{$unit} = [ _sort_positive_first(@{$args{$unit}}) ]; } else { # year day
Bug: DT::Event::Recurrence Modifies Params
I noticed a little bug today. DateTime::Event::Recurrence modifies the parameters that are passed to it. Unfortunately I don't have the time to dig through the code and fix it myself. Is there a defect tracking system that I should log this into? btw - Dave, Flavio, etc, thanks for all of your hard work. In the project I'm working on it has saved countless hours! Example: use DateTime::Event::Recurrence; use Data::Dumper; my @days = qw(1 2 3 4 5); print Dumper([EMAIL PROTECTED]); DateTime::Event::Recurrence-weekly(days = [EMAIL PROTECTED]); print Dumper([EMAIL PROTECTED]); $VAR1 = [ '1', '2', '3', '4', '5' ]; $VAR1 = [ 0, 1, 2, 3, 4 ];