|
Posted by pgodfrin on February 25, 2008, 10:43 am
Please log in for more thread options >
>
>
> > Here are the results:
> > package Mymod;
> > use strict;
> > use warnings;
> > our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
>
> Don't declare variables up-front like this. Perl isn't C or Pascal.
> Declare them as you first need them.
>
> > require Exporter;
> > @ISA = qw(Exporter);
> > @EXPORT = qw(setrun);
>
> > sub setrun
> > {
> > my($evf)=@_ ;
>
> Your code will be much easier to follow if you indent properly.
>
> > my($RVARS,$e,$v);
> > open $RVARS,"<","$evf" or die "Can't open input: $evf\n";
> > foreach (<$RVARS>)
> > {
> > if (($e,$v) = /^export\s+(\w+)=(.+)/)
>
> If you had used
>
> if (my ($e, $v) = ...) {
>
> as I originally had, $e and $v would be scoped to the 'if' statement. It
> is always best to give variables as small a scope as you can.
>
> > {
> > for ($v)
> > {
> > s/(?<!\)$(\w+)/$ENV/g;
> > s/\(.)/$1/g;
>
> To expand these two a little more (note the use of /x, which allows
> whitespace and comments in the 'pattern' part):
>
> s/ # replace...
>
> (?<! \ ) # not immediately after a backslash
> # (this prevents '$foo' from being replaced)
>
> $ # a dollar sign
>
> (\w+) # an identifier, which will be captured into $1
>
> /$ENV/gx; # ...with the appropriate env var
>
> s/ # replace...
> \ # a backslash, followed by
> (.) # any character (captured into $1)
> /$1/gx; # ...with that character (so \->\, $->$, etc.)
>
> > }
> > $ENV=$v;
> > }
> > } # end foreach
>
> Decent indentation makes comments like this redundant.
>
> > close $RVARS;
>
> Since you don't check the return value of close (not usually useful on
> filehandles opened for reading, anyway), there's no point in explicitly
> calling it. Just let the variable go out of scope, and Perl will close
> it for you.
>
> > return 1;
>
> There's no need for a return value like this that doesn't convey any
> information to the caller. Falling off the end of a sub is a perfectly
> respectable way to return (the return value will in fact be the value of
> the last statement executed, but since you don't check it anyway that
> doesn't matter). If you do want a 'null' return, a simple
>
> return;
>
> will return undef in scalar context and the empty list in list context,
> which is usually appropriate.
>
> > } # end setenv
> > 1;
>
> > #!/usr/bin/perl
> > use warnings;
> > no warnings 'once';
> > use Env;
>
> I would recommend against using Env, unless you're a shell programmer
> trying hard not to really learn Perl (which is OK, of course, but don't
> expect a lot of sympathy from the people here :) ). Importing an
> unspecified list of variables into your namespace just for the sake of
> avoiding a hash lookup is not a good tradeoff. (Shell should be avoided
> for much the same reason, neat though the idea is.) The fact that you
> have to turn off 'once' warnings should alert you to the fact this is
> not necessarily a good idea, though there are of course occasions where
> turning off warnings is the right answer.
>
> > #Env::import;
> > #use MymodSimp2 ;
> > use Mymod;
>
> You will find it makes your life easier later if you use explicit import
> lists where practical. When you come to this code later, it makes it
> much simpler to work out where setrun is defined if you can see it in
> the 'use' line.
>
> use Mymod qw/setrun/;
>
> Also, I presume this is just an example, but you should *really* choose
> a better name than 'Mymod'. I tend to use BMORROW::* for my own private
> modules, so I'd call this something like BMORROW::SetEnv, which means it
> would need to live in a file called SetEnv.pm in a directory BMORROW
> somewhere in @INC.
>
> > setrun("etfile");
> > print "TMPTEST: $ENV\n";
>
> If you set $\ (or use the -l switch on the #! line) Perl will add those
> newlines for you. If you have 5.10 you can use 'say' instead of 'print'
> for the same effect. Also, several lines of print statements usually
> work better as a heredoc:
>
> print <<ENV;
> TMPTEST: $ENV
> TMPTOO: $ENV
> Now with scalar vars...
> TMPTEST: $TMPTEST
> ENV
>
> See how much easier it is to get things lined up when you can see how
> they will appear?
>
> > print "TMPTOO: $ENV\n";
> > print "Now with scalar vars...\n";
> > Env::import;
>
> You do realise Env->import (which is subtly but importantly different
> from Env::import) has already been called, at compile time, by the 'use'
> statement? If it hadn't, the next two lines would never have got past
> 'use strict'.
>
> > print "TMPTEST: $TMPTEST\n";
> > print "TMPTOO: $TMPTOO\n";
> > exit;
>
> Again, there's no need to explicitly exit unless you want to do so
> early. Falling off the end of the program is a perfectly good way out.
>
> Ben
Hi Ben,
Thanks for the advice. Yes this is only example or testing code. And
thanks for explaining the RE - that helps a lot. I wasn't aware that
it was OK to permit file handles to close by going out of scope, nor
that it was ok to leave a routine without a return. I'll follow up on
that a little more, because I really do want to learn Perl better. I'm
neither a perl programmer, nor a shell script-er, but a DBA (since,
like, before there were relational databases!).
So essentially all I want to do is the DBA stuff, using perl instead
of shell, which is why you see me asking questions about environment
variables and executing processes in the background :).
In any case, the reason I predeclared the $VERSION and the @...
subroutines is because I saw that in an example in some discussion of
making perl modules. Could you suggest a definitive doc for creating
modules?
About the use Env and Env::import - my goal for these perl scripts
(programs?) was to make them executable from either the command line,
cron or some other scheduling package. So, I am indeed trying to avoid
a hash lookup, mostly for cosmetic's sake, but also for other DBA's
who have never seen perl and refuse to be brought into the [perl]
fold. It would be easier for them to see $mydir="$HOME" as opposed to
$mydir=$ENV. However, I am in the habit of calling Env qw(HOME);
so as to minimize what I'm using.
But to be honest I really don't fully understand module coding and
"EXPORTing" and what the difference is between use Mymod and use Mymod
qw(setrun) really is. which is why I asked about a definitive doc for
that...
But all in all - thanks very, very much for your help and advice - and
anyway who needs your sympathy! <grin>
regards,
pg
| 
XML Sitemap