On Tue, 11 Feb 2003 20:11:46 -0500, Wiggins D'Anconia wrote:

In addition to Wiggins ...

> foreach my $x (1 .. 255) {
> 
>>                 $a = `ping -q -c 1  192.168.1.$b`;
                                                 ^^

Could it be that $x instead of $b was meant ?!

>>                 $b += 1;

You are simulating the work of the loop, as you let $b increment each loop
and the loop has 255 loops. Forget $b.

BTW, allthough it's possible, it's not a good style to work with variables
$a and $b. They are used in Perl globally, e.g. for sorting. See perldoc
perlvar for details.

>>                 print "192.168.1.$b   ";
>>                 print $a;
>>                 $usage="ping.pl [-n]"
> 
> Missing semi-colon....
> 
>>                 if ($arg = "-n") {$e = 0}
                            ^

The = is an assignment not a comparator.
To test two variables for numerical equalness use ==,
for string equalness use eq.
In your case, it's of course
if ($arg eq "-n").
See again perldoc perlop for details.

Where does $arg come from.
If you meant the (first) argument passed at the command line,
it would be
$ARGV[0].

>>                 else {$e = 1}

A shorter (and getting more skilled also more readable) way, to set a
variable to one of two values depending on a condition is the following
idiom:
my $e = ($ARGV[0] eq '-n') ? 0 : 1;

>>                 if ($e = 1) {
                          ^

Again you meant == instead of =.

But if the only sense of $e is to flag whether the argument is -n or not,
why not write it directly:

if ($ARGV[0] eq '-n') {

>>                         open (LOG, ">>/perl/pinglog.txt";
> 
> Missing parentheses. And you should always check to see if open succeeded:
> 
> open(LOG,">>/perl/pinglog.txt") or die "Can't open log for appending: $!";
> 
>>                         print `date`;

Perl can do it also for you:
print scalar localtime;

>>                         print $a;
>>                         }
>>                 else {}

You needn't write the else part if it is empty.
Just leave it izt.

>>                 }
>> 
> 
> While not using semi-colons at the end of blocks is syntactically 
> correct, IMHO it is a very *bad* habit to get into. As it leads to the 
> above.
> 
> Couple other things,
> 
> 1. Name your variables something useful, you have x, b, a, e, and arg, 
> all of which tell yourself and future readers nothing about what they 
> are doing.
> 
> 2.  Theoretically your script could open and close the same file to 
> append to 255 times, you should check to see if you need to open the log 
> before the foreach. Then print to the log if need be during the foreach, 
> but don't reopen it, especially since you aren't closing it, which 
> brings us to.
> 
> 3.  Remember to close your open files when you are done with them.
> 
> Are you in a learning mode, or writing production code?


Best Wishes,
Janek

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

Reply via email to