Beefy Boxes and Bandwidth Generously Provided by pair Networks
Don't ask to ask, just ask
 
PerlMonks  

Module Code Review

by TStanley (Canon)
on Sep 15, 2001 at 03:58 UTC ( #112582=perlquestion: print w/ replies, xml ) Need Help??
TStanley has asked for the wisdom of the Perl Monks concerning the following question:

Lately I have been using a particular set of code over and over again, and I finally decided to put it into a module. I would like everyone's overall opinion of this code, and whether or not they think it might even be worthy of CPAN.
package Date::Lastday; $VERSION=0.01; use strict; ## GLOBALS ## my %Max_Days=("Jan"=>"31","Feb"=>"28","Mar"=>"31","Apr"=>"30", "May"=>"31","Jun"=>"30","Jul"=>"31","Aug"=>"31", "Sep"=>"30","Oct"=>"31","Nov"=>"30","Dec"=>"31"); my %Months=("01"=>"Jan","02"=>"Feb","03"=>"Mar","04"=>"Apr", "05"=>"May","06"=>"Jun","07"=>"Jul","08"=>"Aug", "09"=>"Sep","10"=>"Oct","11"=>"Nov","12"=>"Dec"); ## Functions ## sub new{ my $self=shift; return bless {} $self; } sub last{ my $self=shift; my ($Day,$Month,$Year)=(localtime)[3,4,5]; $Month=sprintf("%02d,($Month+1)); $Year=sprintf("%04d,($Year+1900)); ## Provide for leap years ## my $Leap=$Year % 4; if($Leap==0){ $Max_Days{"Feb"}=29; } if($Day==1 and $Month==1){ # Cover January 1st $Month=12; $Day=31; $Year=$Year-1; }elsif($Day==1){ # Cover the 1st of any month $Month-=1; $Month=sprintf("%02d",$Month); $Day=$Max_Days{$Months{$Month}}; }else{ $Day-=1; $Day=sprintf("%02d",$Day); } return $Month,$Day,$Year; } sub cal{ my $self=shift; my ($Day,$Month,$Year)=(localtime)[3,4,5]; $Month=sprintf("%02d,($Month+1)); $Year=sprintf("%04d,($Year+1900)); ## Provide for leap years ## my $Leap=$Year % 4; if($Leap==0){ $Max_Days{"Feb"}=29; } if($Day==1 and $Month==1){ # Cover January 1st $Month="Dec"; $Day=31; $Year=$Year-1; }elsif($Day==1){ # Cover the 1st of any month $Month-=1; $Month=sprintf("%02d",$Month); $Month=$Months{$Month}; $Day=$Max_Days{$Months}; }else{ $Day-=1; $Day=sprintf("%02d",$Day); $Month=$Months{$Month}; } return $Month,$Day,$Year; } 1; __END__; =pod =head1 NAME Date::Lastday - Perl extension to calculate previous day's date =head1 SYNOPSIS use Date::Lastday; my $Last=new Date::Lastday; my ($Month,$Day,$Year)=$Last->last(); ## OR ## my ($Month,$Day,$Year)=$Last->cal(); =head1 DESCRIPTION Date::Lastday is used to calculate the date of the previous day. It us +es the localtime function of Perl to get the date, then manipulate it + from there. The last function returns the month and day as 2 digit numbers, and th +e year as a 4 digit number. If you wish to have the month returned as + the 3 letter abbreviation (i.e. Jan, Feb, Mar, etc.), use the cal fu +nction. =head2 EXPORT None by default =head1 AUTHOR Thomas Stanley Thomas_J_Stanley@msn.com =head1 COPYRIGHT Copyright (C) 2001 Thomas Stanley. All rights reserved. This program i +s free software; you can redistribute it and/or modify it under the s +ame terms as Perl itself. =head1 SEE ALSO perl(1) =cut

TStanley
--------
There's an infinite number of monkeys outside who want to talk to us
about this script for Hamlet they've worked out
-- Douglas Adams/Hitchhiker's Guide to the Galaxy

Comment on Module Code Review
Download Code
Re: Module Code Review
by japhy (Canon) on Sep 15, 2001 at 04:24 UTC
    Your leap year calculation is incorrect. 2004 is a leap year, 2000 is a leap year, 1904 is a leap year, but 1900 is NOT. The rule is:
    • divisible by 4, and
    • not divisible by 100, or
    • divisible by 400
    if (!($y % 4) and ($y % 100 or !($y % 400))) { ... }
    Other than that, here's a much simpler method for finding out yesterday:
    use Time::Local; my $noon = timelocal(0,0,12, (localtime)[3,4,5]); my ($d,$m,$y) = (localtime($noon - 86400))[3,4,5];

    _____________________________________________________
    Jeff[japhy]Pinyan: Perl, regex, and perl hacker.
    s++=END;++y(;-P)}y js++=;shajsj<++y(p-q)}?print:??;

      You don't even need Time::Local,

      my $yesterday= do { my($y,$m,$d)= (localtime( time-60*60*(12+(localtime)[2]) ))[5,4,3]; sprintf "%04d-%02d-%02d", 1900+$y, 1+$m, $d; };
      or, just for fun:
      my $yesterday= join"-",mapcar{sprintf "%0".shift()."d",pop()+pop()} [4,2,2],[(localtime( time-60*60*(12+(localtime)[2]) ))[5,4,3]],[1900,1,0];

              - tye (but my friends call me "Tye")
Re: Module Code Review
by princepawn (Parson) on Sep 15, 2001 at 05:21 UTC
    The datetime@perl.org mailing list is reworking the entire Date/Time infrastructure now.

    Post your code there for a thorough critique.

Re: Module Code Review
by merlyn (Sage) on Sep 15, 2001 at 05:34 UTC
    I'm not sure why you just didn't use Date::Calc, particularly the Days_in_Month and Calendar functions.

    Using an object here seems a bit of an overkill. I'm not sure I'd ever subclass this, and there's no internal state that might change over time.

    -- Randal L. Schwartz, Perl hacker

Re: Module Code Review
by TStanley (Canon) on Sep 16, 2001 at 05:09 UTC
    Thank you all for your honest critiques. The reason I did a lot of this was primarily a learning experience for myself, especially in the area of object oriented programming. Learning how to put a module together and using it, is basically what I was trying to accomplish. And if the material didn't exist on CPAN (fat chance obviously :-) ), I would gladly add it to the repository.

    TStanley
    --------
    There's an infinite number of monkeys outside who want to talk to us
    about this script for Hamlet they've worked out
    -- Douglas Adams/Hitchhiker's Guide to the Galaxy

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://112582]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others examining the Monastery: (9)
As of 2014-08-01 03:19 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (256 votes), past polls