Re^2: using -T doesn't work

Thanks for the help, Im slowly revising the code - I want to understand the code Im replacing before I replace it, so - so far this is where im at with the code.

use strict;
use warnings;
use Net::Ping;
use CGI qw/:standard/;
use POSIX qw(strftime);

# Clean up our UNIX environment
# for more infor read perldoc perlsec
$ENV{'PATH'} = '/bin:/usr/bin';
delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};

my (@hosts, @uphosts, @downhosts);

if ( -e "ship.cfg" ) {
        open (CFGFILE, "ship.cfg") || die $!;
        while (my @TMP=<CFGFILE>) {
                #if ( $_ =~ /^#/ ) { next }
                # Lets untaint the data after making sure it only matc
+hes
                # word characters... For more info read perldoc perlse
+c.
                foreach $_(@TMP) {
                        if ($_ =~ /^([\w.]+)$/) {
                                $_ = $1;
                                push (@hosts, $1);
                        } else {
                                next;
                        }
                }
        }
        close (CFGFILE);
} else {
        # or enter your hosts here...
        # This is the most secure/preferred way to do this
        @hosts = qw/sapdev0 sapprd0 saptst0 pcbackup vader/;
}

# I haven't decided what else to do here - it just semantics...
my @verbage=("are", "hosts", "are", "hosts");

# udp is default
# other values can be icmp (if youre root)
# or TCP, which is expensive on the wire
my $p = Net::Ping->new('udp');

# Iterate through my hosts and get their status
push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts;
$p->close();

print header, start_html;

print "<tt>\n";
print "\n<center><h3>MSHIP - my status of host IP's</h3></center>\n";
my $now=strftime "%m/%d/%Y %H:%M:%S", (localtime);
print "<center>$now</center>\n";
print hr;

# Just cleaning up the verbage is are host hosts etc...
if (scalar(@downhosts) == 1 ) {
        $verbage[2]="is";
        $verbage[3]="host"
if (scalar(@uphosts) == 1 ) {
        $verbage[0]="is";
        $verbage[1]="host";
}

# We don't care about things that don't exist...
unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
+(@downhosts),"  $verbage[3] unreachable\n",br }
print "There $verbage[0] ",scalar(@uphosts)," $verbage[1] alive\n";

print p,"The following $verbage[1] $verbage[0] alive: \n",br;
foreach my $uitem (sort @uphosts) { # sort is for cleanliness...
        print li("$uitem\n"),br;
}
# Again, we don't care about things that don't exist
unless (scalar(@downhosts) == 0) { print p,"The following $verbage[3] 
+$verbage[2] unreachable: \n",br }
foreach my $ditem (sort @downhosts) {
        print li("$ditem\n"),br;
}
print "</tt>\n";

print start_form,
        p,submit('Refresh'),
        end_html;
[download]

Where Im at now is on this piece of code:

# Iterate through my hosts and get their status
push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts;
$p->close();
[download]

what I get from this is if ping then either @uphosts (if successful) or @downhosts gets $_ from @host. Im just not getting why is it in the form of
@{ $p->ping($_) ? \@uphosts : \@downhosts }
and what does referencing @downhosts and @uphosts do that just calling them won't do?
Or where can I read more on this other than perldata, perlref, etc...
Thanks
Ted

Comment on Re^2: using -T doesn't work Select or Download Code

Replies are listed 'Best First'.
Re^3: using -T doesn't work by blazar (Canon) on Dec 30, 2004 at 14:16 UTC
Thanks for the help, Im slowly revising the code - I want to understand the code Im replacing before I replace it, so - so far this is where im at with the code. Ok, Please note that to distinguish my code from yours, mine will be commented out (when in a paragraph by itself). Note: the code I proposed the last time was rewritten from yours according to my own personal preferences. I tried to keep its general structure as close as possible to yours, though. `my (@hosts, @uphosts, @downhosts); if ( -e "ship.cfg" ) { open (CFGFILE, "ship.cfg") \|\| die $!;` [download] If you use a lexical filehandle then you do not need to close() it (when going out of scope (and if there are no more references to it)). Personally I like to avoid all parentheses altogether and use the low priority C<and> for flow control. I like to use the three-args from of open() even if it is not strictly necessary for I think it's a good habit. `while (my @TMP=<CFGFILE>) {` [download] Perl's idiom for reading "lines" (whatever these can be according to $/) is `# while (<CFGFILE>) {` [download] If you want to do exactly the same with an explicit variable rather than $_, then you have to do `# while (defined(my $line = <ARGV>)) { # ... # }` [download] What you wrote won't do what you think, since notwithstanding the C<while>, all of the file's content will be read into @TMP at a time. `#if ( $_ =~ /^#/ ) { next } # commented out in the OP` [download] And $_ is? Also, while legal, you're missing the whole point about $_: it's the implicit variable on which so many operators and functions act. So you either want to `$variable =~ /something/` or `/something/` (if your variable is $_). Also, you may not like statement modifiers, but if there's a case in which one would come really handy is exactly this one: `next if /^#/;` (IMHO much more readable/immediate). `foreach $_(@TMP) {` [download] Don't! Quite similarly you either do something `for my $var (@list) { ...` or `for (@list) { ...` (if you want to use $_). `if ($_ =~ /^([\w.]+)$/) {` [download] Don't! `$_ = $1;` [download] Why?!? `push (@hosts, $1); } else { next; }` [download] No need for the C<next> block, as if the condition is not fulfilled it will pass to the next iteration anyway. `close (CFGFILE);` [download] If you use a lexical filehandles then you do not need to close() them. `# Just cleaning up the verbage is are host hosts etc... if (scalar(@downhosts) == 1 ) { $verbage[2]="is"; $verbage[3]="host" if (scalar(@uphosts) == 1 ) { $verbage[0]="is"; $verbage[1]="host"; }` [download] As I wrote the other time, no need for scalar() there, and you can use an array slice. Not to say that this is not correct, strictly speaking, only IMHO it is much cleaner the other way round. For an actual example see the sample code I posted in the other node. `# We don't care about things that don't exist... unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar` [download] No need for the first scalar() as well as no need for the C<==0>. (No: this is not a mouse!! ;-) The same cmts apply to much of the following code, I'm not repeating them. Also, this is yet another one of those things that depend largely on personal preferences, but well written code should be self explanatory and only really relevant comments should be there. Commenting the obvious doesn't add to readability, IMHO: it has just the opposite effect instead! Where Im at now is on this piece of code: # Iterate through my hosts and get their status push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts; $p->close(); what I get from this is if ping then either @uphosts (if successful) or @downhosts gets $_ from @host. Im just not getting why is it in the form of @{ $p->ping($_) ? \@uphosts : \@downhosts } Well, it's just a way to do it that I find to be convenient. Of course it's up to your personal preferences. You may consider it to be slightly ineffcient, but we're dealing with only a bunch of them anyway and `$p->ping($_)` takes most of the time here. and what does referencing @downhosts and @uphosts do that just calling them won't do? Or where can I read more on this other than perldata, perlref, etc... Hint: push() wants an array as its first argument, I'm giving it one. I think perlref has all that is needed to be known. Check perlreftut, as well.	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re^3: using -T doesn't work
by blazar (Canon) on Dec 30, 2004 at 14:16 UTC

Thanks for the help, Im slowly revising the code - I want to understand the code Im replacing before I replace it, so - so far this is where im at with the code.

Note: the code I proposed the last time was rewritten from yours according to my own personal preferences. I tried to keep its general structure as close as possible to yours, though.

my (@hosts, @uphosts, @downhosts);

if ( -e "ship.cfg" ) {
        open (CFGFILE, "ship.cfg") || die $!;
[download]

Personally I like to avoid all parentheses altogether and use the low priority C<and> for flow control. I like to use the three-args from of open() even if it is not strictly necessary for I think it's a good habit.

        while (my @TMP=<CFGFILE>) {
[download]

        # while (<CFGFILE>) {
[download]

        # while (defined(my $line = <ARGV>)) {
        #     ...
        # }
[download]

                #if ( $_ =~ /^#/ ) { next }  # commented out in the OP
[download]

Also, while legal, you're missing the whole point about $_: it's the implicit variable on which so many operators and functions act. So you either want to $variable =~ /something/ or /something/ (if your variable is $_).

Also, you may not like statement modifiers, but if there's a case in which one would come really handy is exactly this one: next if /^#/; (IMHO much more readable/immediate).

                foreach $_(@TMP) {
[download]

Don't!

for my $var (@list) { ...

for (@list) { ...

                        if ($_ =~ /^([\w.]+)$/) {
[download]

Don't!

                                $_ = $1;
[download]

Why?!?

                                push (@hosts, $1);
                        } else {
                                next;
                        }
[download]

        close (CFGFILE);
[download]

# Just cleaning up the verbage is are host hosts etc...
if (scalar(@downhosts) == 1 ) {
        $verbage[2]="is";
        $verbage[3]="host"
if (scalar(@uphosts) == 1 ) {
        $verbage[0]="is";
        $verbage[1]="host";
}
[download]

# We don't care about things that don't exist...
unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
[download]

No need for the first scalar()

The same cmts apply to much of the following code, I'm not repeating them.

Also, this is yet another one of those things that depend largely on personal preferences, but well written code should be self explanatory and only really relevant comments should be there. Commenting the obvious doesn't add to readability, IMHO: it has just the opposite effect instead!

Where Im at now is on this piece of code: # Iterate through my hosts and get their status push @{ $p->ping($_) ? \@uphosts : \@downhosts }, $_ for @hosts; $p->close();
what I get from this is if ping then either @uphosts (if successful) or @downhosts gets $_ from @host. Im just not getting why is it in the form of @{ $p->ping($_) ? \@uphosts : \@downhosts }

$p->ping($_)

and what does referencing @downhosts and @uphosts do that just calling them won't do? Or where can I read more on this other than perldata, perlref, etc...

[reply]
[d/l]
[select]