improve subroutine.. seems klunky

M

matg

I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.

Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..

the code:...................

###########################
#
# creates timestamp from current date
# adds a leading 0 to the month and day string
# and works on previous day
#
###########################
sub create_timestamp{

($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = gmtime(time);
if ($year == "102"){;
$year = "2002";
}
if ($year == "103"){;
$year = "2003";
}
if ($year == "104"){;
$year = "2004";
}
if ($year == "105"){;
$year = "2005";
}
if ($year == "105"){;
$year = "2005";
}
if ($year == "106"){;
$year = "2006";
}
if ($mon== "0"){
$month = "01"
}
if ($mon== "1"){
$month = "02"
}
if ($mon== "2"){
$month = "03"
}
if ($mon== "3"){
$month = "04"
}
if ($mon== "4"){
$month = "05"
}
if ($mon== "5"){
$month = "06"
}
if ($mon== "6"){
$month = "07"
}
if ($mon== "7"){
$month = "08"
}
if ($mon== "8"){
$month = "09"
}
if ($mon== "9"){
$month = "10"
}
if ($mon== "10"){
$month = "11"
}
if ($mon== "11"){
$month = "12"
}
if ($mday > 10){
$previous_day=($mday-1)
}
if ($mday == "2") {
$previous_day="01";
}
if ($mday == "3") {
$previous_day="02";
}
if ($mday == "4") {
$previous_day="03";
}
if ($mday == "5") {
$previous_day="04";
}
if ($mday == "6") {
$previous_day="05";
}
if ($mday == "7") {
$previous_day="06";
}
if ($mday == "8") {
$previous_day="07";
}
if ($mday == "9") {
$previous_day="08";
}
if ($mday == "10") {
$previous_day="09";
}
$timestamp = "$month$previous_day";
}
 
I

Ian Wilson

matg said:
I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.

Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..

the code:...................

###########################
#
# creates timestamp from current date
# adds a leading 0 to the month and day string
# and works on previous day
#
###########################
sub create_timestamp{

See the posting guidelines. "use strict;" and "use warnings;" might have
shed some light on a few things.
($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = gmtime(time);
if ($year == "102"){;
"==" is a numeric comparison, "102" is a string. Use "eq" for string
comparisons. However you *should* be treating the variables as numbers.

$year += 1900;
$year = "2002";
}
if ($year == "103"){;
$year = "2003";
}
if ($year == "104"){;
$year = "2004";
}
if ($year == "105"){;
$year = "2005";
}
if ($year == "105"){;
$year = "2005";
}
if ($year == "106"){;
$year = "2006";
}

All the above are redundant if you simply add 1900 to the year.
if ($mon== "0"){
$month = "01"
}
if ($mon== "1"){
$month = "02"
}
if ($mon== "2"){
$month = "03"
}
if ($mon== "3"){
$month = "04"
}
if ($mon== "4"){
$month = "05"
}
if ($mon== "5"){
$month = "06"
}
if ($mon== "6"){
$month = "07"
}
if ($mon== "7"){
$month = "08"
}
if ($mon== "8"){
$month = "09"
}
if ($mon== "9"){
$month = "10"
}
if ($mon== "10"){
$month = "11"
}
if ($mon== "11"){
$month = "12"
}
$mon++;

if ($mday > 10){
$previous_day=($mday-1)
}
if ($mday == "2") {
$previous_day="01";
}
if ($mday == "3") {
$previous_day="02";
}
if ($mday == "4") {
$previous_day="03";
}
if ($mday == "5") {
$previous_day="04";
}
if ($mday == "6") {
$previous_day="05";
}
if ($mday == "7") {
$previous_day="06";
}
if ($mday == "8") {
$previous_day="07";
}
if ($mday == "9") {
$previous_day="08";
}
if ($mday == "10") {
$previous_day="09";
}

The obvious (but still incorrect) way to do this is
$previous_day = sprintf("%.2d", $mday-1);
What happens if today is the first of the month?

If doing date arithmetic, I'd use the date modules Date::Manip or
Date::Calc.

The answer is in 'perldoc -q yesterday' and in the Perl FAQ 4.17 often
posted here.
 
B

Brian Wakem

matg said:
I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.

Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..

the code:...................
<snip very longwinded code>


my ($day,$month) = (localtime(time-86400))[3,4];
my $timestamp = sprintf "%02d%02d",$month+1,$day;
 
M

matg

wow - thanks to all for the replies .. I'll have to pay more
attention.. and read more first....

again many thanks
 
I

Ian Wilson

Brian said:
matg wrote:

I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.

Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..

the code:...................

<snip very longwinded code>


my ($day,$month) = (localtime(time-86400))[3,4];
my $timestamp = sprintf "%02d%02d",$month+1,$day;

Doesn't that give the wrong answer for two hours every year in locales
which have DST?
 
B

Brian Wakem

Ian said:
Brian said:
matg wrote:

I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.

Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..

the code:...................

<snip very longwinded code>


my ($day,$month) = (localtime(time-86400))[3,4];
my $timestamp = sprintf "%02d%02d",$month+1,$day;

Doesn't that give the wrong answer for two hours every year in locales
which have DST?


Probably.

This should avoid DST issues -


use Time::Local;
my ($day,$month,$year) = (localtime(time))[3..5];
my $time = timelocal(0,0,12,$day,$month,$year);
($day,$month) = (localtime($time-86400))[3,4];
my $timestamp = sprintf "%02d%02d",$month+1,$day;
 
A

axel

matg said:
I've cobbled together a subroutine to get a timestamp and create a
further string that represents the previous day.
Can anyone improve on this .. it seems very klunky.. and I'm sure
there's a neater way to do things..
###########################
#
# creates timestamp from current date
# adds a leading 0 to the month and day string
# and works on previous day
#
###########################
sub create_timestamp{

($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = gmtime(time);
if ($year == "102"){;
$year = "2002";
}
if ($year == "103"){;
$year = "2003";
}

Why not just:

$year += 1900;

But then you might as well add on an extra 753 to to base the year
on A.U.C. (Ab urbe condita) since you never actually use the year
in the timestamp.
if ($mon== "0"){
$month = "01"
}

Again

$month = $mon + 1;

And worry about the formatting when you build the timestamp. Actually
you could omit this step entirely... see below.
if ($mday > 10){
$previous_day=($mday-1)
}
if ($mday == "2") {
$previous_day="01";
}

So when the $mday is 1, it just gets ignored. An interesting approach to
the problem.
$timestamp = "$month$previous_day";

This should then be built from something along the lines of:

$timestamp = sprintf ("%02d:%02d\n", $mon + 1, $previous_day);

But far more easier would be to use a CPAN module... maybe
Date::Format...

use Date::Format;
print time2str("%Y:%m:%d", time);

prints

2005:06:10

and if you subtract 86400 seconds from the time given to time2str, you
should get the previous day.

Axel
 
T

Tad McClellan

Ian Wilson said:
matg wrote:
^
^
^ What's that for?

"==" is a numeric comparison, "102" is a string. Use "eq" for string
comparisons. However you *should* be treating the variables as numbers.


So, it should either be:

if ($year == 102) { # treat $year as a number

or

if ($year eq '102') { # treat $year as a string
 
A

axel

James David said:
my ($day,$month,$year) = (localtime(time))[3..5];
^^^^^^^^^^^^^^^^^
I'm almost embarrassed to ask, but could someone explain what's happening in
this statement. Thanks.
More specifically, I'm trying to understand the theory or the construct of
the placement of [3..5] adjacent to localtime() to extract the needed
elements. Thanks again.

Think of it as applying an array slice to the list returned by
localtime().

Axel
 
B

Brian Wakem

James said:
James David said:
my ($day,$month,$year) = (localtime(time))[3..5];
^^^^^^^^^^^^^^^^^

I'm almost embarrassed to ask, but could someone explain what's happening in
this statement. Thanks.

More specifically, I'm trying to understand the theory or the construct of
the placement of [3..5] adjacent to localtime() to extract the needed
elements. Thanks again.


localtime(time) returns an array that is commonly split into its parts like
this -

my ($sec, $min, $hour, $mday, $month, $year, $wday, $yday, $isdst) =
localtime(time);

[3..5] selects just the 4th,5th and 6th elements of the array, which are
then assigned to ($day,$month,$year).

... being the range operator.
 
J

John Bokma

Tad said:
So, it should either be:

if ($year == 102) { # treat $year as a number

or

if ($year eq '102') { # treat $year as a string

Moreover, I would recommend adding 1900 to the year first, so it's more
clear that you mean 2002.
 
T

Tad McClellan

Brian Wakem said:
James said:
James David said:
<snip>

my ($day,$month,$year) = (localtime(time))[3..5];
I'm almost embarrassed to ask, but could someone explain what's happening in
this statement. Thanks.

More specifically, I'm trying to understand the theory or the construct of
the placement of [3..5] adjacent to localtime() to extract the needed
elements. Thanks again.


localtime(time) returns an array


Functions in Perl never return an "array", they return a "list".

that is commonly split into its parts like
this -

my ($sec, $min, $hour, $mday, $month, $year, $wday, $yday, $isdst) =
localtime(time);

[3..5] selects just the 4th,5th and 6th elements of the array,


And that is known as a "list slice". The syntax for a list slice is
"stuff in parenthesis followed by stuff in square brackets".

The parens stuff is the list to be sliced, and the square stuff
is a list of the indexes that you want to slice out.


See the "Slices" section in

perldoc perldata

for info on all three kinds of slices (array slice and hash slice being
the other two).
 
M

Mark Seger

The below code would correctly subtract one day from current gmt date:
use Date::Calc qw(Today Add_Delta_Days);
($year,$month,$day) = Add_Delta_Days(Today(1), -1);
$timestamp = sprintf "%02d%02d", $month, $day;
This whole thread also reminded me of something I stumbled on with
'time' a long time ago and I'm still not sure if it's a bug or a feature
and that has to do with how perl treats DST in general. It all happened
long ago when I had written a routine that reported the number of
seconds between 2 times and when DST was in the middle it reported the
wrong number!

While I can appreciate on one level if you adjust the clock you
sometimes 'lose' an hour and sometimes 'gain' one, but aren't the actual
number of seconds that passed still 86400? Clearly there are 2 'time's
involved, one which is the number of seconds since the epoch and the
other is the number of seconds that representst the current time and
perl uses the same for both which simply cannot be right. I suspect
that's probably what the O/S does, unless someone already knows.

Perhaps another way to look at this is with the following script which
starts 100 days ago (because I wasn't sure which date the times change
since they differ all over the world) and also because I'm lazy and
computers are fast. Basically it gets 2 concecutive dates (but it will
break if you run right around DST, but that's a different topic) and
uses those to get the timestamps at midnight on both days. It then
reports the number of seconds in the first day.

#!/usr/bin/perl -w

use Time::Local;

$start=time-86400*100;
for ($i=$start;; $i+=86400)
{
($secs1, $mins1, $hour1, $day1, $mon1, $year1)=localtime($i);
($day2, $mon2, $year2)=(localtime($i+86400))[3..5];

$seconds1=timelocal(0, 0, 0, $day1, $mon1, $year1);
$seconds2=timelocal(0, 0, 0, $day2, $mon2, $year2);

printf "%4d%02d%02d %02d:%02d:%02d SECS: %d\n", $year1+1900,
$mon1+1, $day1,\
$hour1, $mins1, $secs1, $seconds2-$seconds1;
}

-mark
 
R

RedGrittyBrick

Mark said:
Clearly there are 2 'time's
involved, one which is the number of seconds since the epoch and the
other is the number of seconds that representst the current time
perl -e "print time"
1118586568

a) The number of seconds since the epoch is 1118586568
b) The number of seconds that represent the current time is 1118586568

Looks like one time to me.
 
M

Mark Seger

1118586568

a) The number of seconds since the epoch is 1118586568
b) The number of seconds that represent the current time is 1118586568

Looks like one time to me.

yes, of course you are right. the point I was trying to make about time
calculations is that if you look at the difference between two days at
the same time the number of seconds involved is not a mulitple of 86400
if you cross a time change. sorry about that...
-mark
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,596
Members
45,143
Latest member
DewittMill
Top