Andrew Gaffney wrote:

>
> My code ended up looking like:

use strict;
use warnings;
use constant PAY_PERIOD_DAYS => 14;

>
>
> my @payperiods;

Underscores are both permissible and advisable in variable names:
my @pay_periods;

>
> my @finalpayperiods;
> my $lastperiodend = Date::Simple->new('2004-01-10');
> while() {
>    my $newstart = $lastperiodend + 1;

Good enough.  $new_start should take the value following  $last_period_end.

>    my $newend = $newstart + 13;

This works, but could be more clear.  The number 13 here is sort of a "magic
number", which requires reading contextual code to understand.  Better to use
the size of the pay period
my $new_end = $last_period_end + 14;
or
my $new_end = $last_period_end + PAY_PERIOD_DAYS;

>
>    my @lt = localtime;
>    last if(Date::Simple->new($lt[5]+1900, $lt[4]+1, $lt[3]) < $newstart);

This probably does what you want, but what does it mean?  It may seem strange to
say this about a single line, but I think this should be in a function of its
own, if only so that it has a logiacl identifier.  Hmmm, actually, this probably
should not be inside the loop at all.  Unless you expect some significant change
in the local time values between iterations of the loop, you should perform the
creation of the new Date::Simple object only once, then compare the object as
you go through the loop.

Better yet, you should have this in the loop control, since controlling the
execution of the loop is its purpose.

while($last_period_end < $current_date - 1) {

>    push @payperiods, "$newstart to $newend";
>    $lastperiodend = $newend;
> }
> my $periodcounter = 0;

=item remove complicated stuff

> foreach(reverse @payperiods) {
>    $periodcounter++;
>    last if($periodcounter > 6);
>    push @finalpayperiods, $_;
> }

=cut

for (1..6) {
    push @final_pay_periods, pop @pay_periods;
}

Joseph


-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>


Reply via email to