Andrew Gaffney wrote:
My code ended up looking like:
use strict; use warnings;
This code is running under mod_perl, so I think both of those are already being used.
use constant PAY_PERIOD_DAYS => 14;
Good idea. I never use constants. My code makes sense to me, but few others for this reason ;)
my @payperiods;
Underscores are both permissible and advisable in variable names: my @pay_periods;
Another good idea. Readability is good.
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.
That line creates a date object for today. It ended up there because I was originally looking at the documentation for v2.03 of Date::Simple when I only have v1.03 installed. There is a today() call in 2.03. So, it was 'last if(Date::Simple::today() < $newstart);'.
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; }
I didn't do it this way because there is a "first" pay period. If there are only 2 pay periods from the starting date, you can't build a list of 6. My way takes that into account. Below is the modified code based on (most of) your suggestions.
use constant PAY_PERIOD_DAYS => 14;
my @pay_periods;
my @final_pay_periods;
my $last_period_end = Date::Simple->new('2004-03-20');
my @lt = localtime;
my $today = Date::Simple->new($lt[5]+1900, $lt[4]+1, $lt[3]);
while($today > $last_period_end + 1) {
my $new_start = $last_period_end + 1;
my $new_end = $last_period_end + PAY_PERIOD_DAYS;
push @pay_periods, "$new_start to $new_end";
$last_period_end = $new_end;
}
my $period_counter = 0;
foreach(reverse @pay_periods) {
$period_counter++;
last if($period_counter > 6);
push @final_pay_periods, $_;
}-- Andrew Gaffney Network Administrator Skyline Aeronautics, LLC. 636-357-1548
-- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>
