Chad Kellerman wrote:
> 
> Hello everyone,

Hello,

>      I think I am getting the hang of using perl, just rough around the
> edges.  I was hoping you guys can give me a hand.  Here is my issue.
> From what I understand using global variables such as my #variable; it
> not the most "proper" way of coding.

Lexical scoping was added to Perl5 in order to facilitate programming in
the large.  Perhaps MJD's article "Coping With Scoping" will help
http://perl.plover.com/FAQs/


> I ran into a problem that I can't
> seem to get around using it. (just because I am a little new at this
> programming stuff)
> 
>  I have a text file that has fields separated by pipes.  Like so
> 
> ----------------------<snip of text file>----------------------------
> 
> #dir name|ip|hostname|back|port|chK|ver|mon|cl|db|pure|usr
> /export6/home213/bob.work|192.168.123.23|bob.com|1|1|1|5|21,22|1|0|1|bob
> /export5/home413/chuck.home|192.168.123.201|chuck.com|1|1|1|3|80|1|1|1|chuck     
>#/export9/home325/sally.home|192.168.123.56|sally.com|1|1|1|3|80,21|1|1|1|sally
> /export1/home425/tom.home|192.168.123.100|tom.com|1|0|1|3|80,21|0|1|1|tom
> The text file has about 1000 entries just like the ones above.
> 
> Here is the script I wrote:
> 
> ----------------------<snip of code>---------------------------------
> 
> #!/usr/bin/perl
> use strict;
> use warnings;
> $|++;
> 
> use FileHandle;
> 
> my $msl_Host;
> my %msl_Host;
> my $serverList = "/usr/local/account/main-server.list";
> 
> &mslSplit($serverList);    # pass the list into the subroutine.
            ^^^^^^^^^^^             ^^^^^^^^
You aren't passing a list but a single scalar.  :-)


> foreach my $key (keys %msl_Host) {
>         my $href = $msl_Host{$key};
>         next if ($href->{'ignore_flag'} =~ "#");
                                          ^^^^^^
This works because the binding operator (=~) converts the string to a
regular expression:

perldoc perlop
[snip]
       Binding Operators
[snip]
       If the right argument is an expression rather than a
       search pattern, substitution, or transliteration, it is
       interpreted as a search pattern at run time.  This can be
       less efficient than an explicit search, because the pat­
       tern must be compiled every time the expression is evalu­
       ated.

However it looks like you want the eq equality operator:

          next if $href->{'ignore_flag'} eq '#';


>         print "$href->{'host_name'}\n";

You could use the %msl_Host variable directly for the two previous
lines.

          next if $msl_Host{$key}->{'ignore_flag'} eq '#';
          print "$msl_Host{$key}->{'host_name'}\n";


> }
> 
> sub mslSplit {
>      my $msl = new FileHandle "@_", "r" || die $?;
                                ^^^^      ^^     ^^
You are stringifying an array when you should be using the first element
of that array ($_[0] instead of "@_".)  You are using the higher
precedence || which will not do what you want it to do here.  You are
using $? in the error message but file open errors are reported in $!. 
If you are using Perl 5.6 or newer you don't need the FileHandle module
to get a lexically scoped file handle.

# Perl 5.6 or newer
      open my $msl, '<', $_[0] or die "Cannot open $_[0]: $!";

# On older Perls using FileHandle
      my $msl = new FileHandle $_[0], 'r' or die "Cannot open $_[0]:
$!";


>      while (<$msl>) {
>             chomp;
>            my ($backup_dir, $host_ip, $host_name, $backup_flag,undef) = split(/\|/) ;
                                                                ^^^^^^

perldoc -f split
[snip]
               The LIMIT parameter can be used to split a line
               partially

                   ($login, $passwd, $remainder) = split(/:/, $_, 3);

               When assigning to a list, if LIMIT is omitted,
               Perl supplies a LIMIT one larger than the number
               of variables in the list, to avoid unnecessary
               work.  For the list above LIMIT would have been 4
               by default.  In time critical applications it
               behooves you not to split into more fields than
               you really need.

So the undef at the end of your list is unnecessary.


>            next if $backup_dir =~ /^#directory name/;
>            my ($comment, undef, $group_dir, $host_dir) = split(/\// ,
> $backup_dir);
>            my $daily_server = substr($group_dir,0,length($group_dir)-2);

Another way to do this.  :-)

            (my $daily_server = $group_dir) =~ s/..$//;


>            my %hostEntry = (
>                   'backup_dir'   => $backup_dir,
>                   'host_ip'      => $host_ip,
>                   'backup_flag'  => $backup_flag,
>                   'group_dir'    => $group_dir,
>                   'daily_server' => $daily_server,
>                   'host_dir'     => $host_dir,
>                   'ignore_flag'  => $comment,
>                   'host_name'    => $host_name
>             );
>             $msl_Host{$host_name} = \%hostEntry;

You could just assign all of this directly:

             $msl_Host{$host_name} = {
                   'backup_dir'   => $backup_dir,
                   'host_ip'      => $host_ip,
                   'backup_flag'  => $backup_flag,
                   'group_dir'    => $group_dir,
                   'daily_server' => $daily_server,
                   'host_dir'     => $host_dir,
                   'ignore_flag'  => $comment,
                   'host_name'    => $host_name
             };


>      }
>      $msl->close;

Because $msl is lexically scoped it will close when the variable goes
out of scope at the end of the subroutine but it never hurts to be
sure.  :-)


> }
> 
> -------------------------</end of code>---------------------------------
> This works exactly the way I want it.  I can change my print statement
> in the foreach loop and print any importatn field that I want.  Ther are
> no additional fields I need.
> 
>     I was wondering what can I do to clean it up. (Maybe be more
> efficient)  I really don't like having:
> 
> my $msl_Host;
> my %msl_Host;
> 
>   declared like that.  But I don't know how to rewrite it so I don't
> have to declare it like that.  I kind stumbled across getting this just
> to work. Any help would be appreciated.

For a relatively small program like this using file scoped variables is
not a big deal.  Some other ways to do it are:

############## pass a reference to a hash ##############
my $serverList = '/usr/local/account/main-server.list';

mslSplit( $serverList, \my %msl_Host );

for my $key ( keys %msl_Host ) {
    ...
    print ...
    }

sub mslSplit {
    my ( $filename, $hashref ) = @_;
    ...
    ...
    %$hashref = ( key => $value, etc... );
    }

############## return a reference to a hash ##############
my $serverList = '/usr/local/account/main-server.list';

my $hashref = mslSplit( $serverList );

for my $key ( keys %$hashref ) {
    ...
    print ...
    }

sub mslSplit {
    my $filename = shift;
    ...
    ...
    return { key => $value, etc... };
    }

############## return an actual hash (not as efficient) ##############
my $serverList = '/usr/local/account/main-server.list';

my %msl_Host = mslSplit( $serverList );

for my $key ( keys %msl_Host ) {
    ...
    print ...
    }

sub mslSplit {
    my ( $filename ) = @_;
    ...
    ...
    return ( key => $value, etc... );
    }



John
-- 
use Perl;
program
fulfillment

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to