panamasnet has asked for the wisdom of the Perl Monks concerning the following question:

Hello all. I have this script that I have hacked, modified, improved and altered over the last few years. It is a Cyber Card Script (postcard- ecard) when I first got a hold of it it was a very simple script. I have added lots of features and things to it. The reason I'm asking for help is that I have a couple of ideas in order to improve it some more but as hard as I have tried it I have had no luck in getting the script to work. This is what I would like to add to it. 1. The ability of sending the card at a later date. 2. The ability of letting someone know when the card that they sent has been picck up. 3. The ability of adding users to the mailing list 4. A faster loading script. You can see the script in action if you visit http://www.panamas.net and send a card. To view the actual script you can go to http://www.panamas.net/card.txt I would apprecite any help that anyone can provide in this matter. Web you all later...Panama http://www.panamas.net

Replies are listed 'Best First'.
Re: A few ideas for an old script
by maverick (Curate) on Dec 26, 2001 at 10:50 UTC
    I would advise some design changes and coding practice improvements.
    • use strict; (this will help catch a lot of potential bugs)
    • use warnings; (this will help catch a few more)
    • use taint mode (this will help make the script more secure)
    • use CGI.pm
    • use Email::Valid or some other CPAN module instead of a home rolled email validator
    • use Mail::Sendmail or some other CPAN module instead of opening a pipe to sendmail
    • Consider dividing into several smaller scripts (on per feature) instead of adding more features to this one. Smaller scripts == faster compile time. This will also improve your ability to maintain this code as well as add new features.
    First glance suggestions...Hope they help.

    /\/\averick
    perl -l -e "eval pack('h*','072796e6470272f2c5f2c5166756279636b672');"

[reply]
Re: A few ideas for an old script
by simon.proctor (Vicar) on Dec 26, 2001 at 18:29 UTC
    A few other suggestions for you:

    1) Replace the opening constants with actual constants

    i.e: $MAX_MAILS=200;
    becomes
    use constant MAX_MAILS => 200;

    2) A fairly minor optimisation but consider replacing all double quoted strings with single quotes whereever possible

    i.e.
    $temp_pic= $BASEURL . "/" .
    becomes
    $temp_pic = BASEURL . '/' (using constants as well :P)

    3) s/\s\t\r\n+//i
    Does this really need to be case insensitive - you are not matching any characters only classes of whitespace. Check all your regexes as this is an unecessary slowdown (I think :P).

    4)Finally, consider using slurp mode for opening any files.
    i.e.(not tested code so watch out!)
    open(FH,"<$file");
my $data;
{
  local $/ = undef;
  $data = <FH>;          # Slurrp
}
close(FH);

    [download]
    I would also consider putting all your sub routines into a module and create what I would term a functionality object. You can then use something like 'autouse' and have a routine for each phase of the program. You don't need to split it up then. You could also consider embedding this via mod_perl and preloading the object within Apache (assuming an apache + mod_perl host)

    Hope that helps! Contact me if you need any more help :)

    Simon
[reply]
[d/l]

Back to Seekers of Perl Wisdom