Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

Re: I know this code could be better...

by jreades (Friar)
on Jun 20, 2001 at 18:50 UTC ( [id://90017]=note: print w/replies, xml ) Need Help??


in reply to I know this code could be better...

my $links = 'h:\perl scripts\links.txt';

Since this is apparently a constant, you might consider rewriting it as:

my $LINKS = 'h:\perl scripts\links.txt';

But if you want to be rigorous and good about preparing for when your script needs to do everything and run the kitchen sink, you might go so far as to write:

sub LINKS { return 'h:\perl scripts\links.txt'; }

While your program remains simple , I believe that this will be inlined by the Perl compiler (so no performance hit), but if your program grows, such that lines might change depending on the context, you only have to rewrite the subroutine and everything else will work as expected.

Actually, looking at your middle sub, I can't see any reason why you wouldn't just suck the file into a scalar and save yourself the overhead of array allocation:

undef $/;

To make things more reader-friendly and more extensible it's also a good idea to pass variables to your subs rather than simply calling variables declared elsewhere:

sub middle { my @output = @_; print while @output; }

This sub now only makes the assumption that it should print whatever you've passed it... so now, it's re-usable and should probably be renamed print_to_user or some such.

DISCLAIMER: I've been stuck as a Java programmer for the past few months so my Perl's getting more than a little rusty (trying to get involved again in Perlmonks), so not all of this stuff may perform as advertised. If so, then I apologize.

Replies are listed 'Best First'.
Re: Re: I know this code could be better...
by petdance (Parson) on Jun 20, 2001 at 22:41 UTC
    Instead of
    sub LINKS { return 'h:\perl scripts\links.txt'; }
    you're better off to be more Perlish with constant.pm:
    use constant LINKS => 'h:\perl scripts\links.txt';

    xoxo,
    Andy

    %_=split/;/,".;;n;u;e;ot;t;her;c; ".   #   Andy Lester
    'Perl ;@; a;a;j;m;er;y;t;p;n;d;s;o;'.  #   http://petdance.com
    "hack";print map delete$_{$_},split//,q<   andy@petdance.com   >
    

      Yes, but consider a change needing to be made down the line:

      Q: "Hey petdance we're porting this script to Unix, can you make it run there too?"

      A: "Sure, I'll just change the constant."

      Q: "Hey what did you change in that script so that now it doesn't work on NT anymore?"

      A: "Hmmmmm"

      Ok, so the example is a stretch, but imagine the solution using a sub instead of a constant.

      sub LINKS { if (sub_to_determine_OS() =~ /(Unix|Linux)/) { return "/u/jreades/links.txt"; } else { return "U:\jreades\nt_links.txt"; } }

      And all of that requires exactly no changes to the rest of your application.

      Yes, it is paranoid, but that doesn't mean they're not out to get you.

Re: Re: I know this code could be better...
by Kevman (Pilgrim) on Jun 21, 2001 at 14:02 UTC
    I generally like to pass in locations with either environment variables or thru command line options. Saves on going back through all your code when the directory structure changes... Although some may disagree with me :)

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://90017]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (2)
As of 2024-04-26 04:15 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found