vectorvillain has asked for the wisdom of the Perl Monks concerning the following question:

Can someone tell me what's wrong with
@openip = split(/./,$stip);
in the folling script:
#!/usr/bin/perl
# IP ranger
open(IPFILE,"iplist.txt");
while (<IPFILE>) {
    ($stip,$edip) = split(/:/,$_);
    chomp ($edip);
    print "$stip\n";
    print "$edip\n";
    @openip = split(/./,$stip);
    $count = @openip;
    print "$count\n";
#    for ($i = 0; $i <= 4; $i++) {
#        print "@srtip\n";
#    };
};
close(IPFILE);

[download]

the contents of iplist.txt:
10.1.2.2:10.1.2.200
EOF

the output is:
10.1.2.2
10.1.2.200
0


I've run it on AcitvePerl and Perl v5.005_03 and v5.6.0 for linux. I would appreceate any help, Thanks guys.

Vec

Replies are listed 'Best First'.
Re: beginner syntax question
by dragonchild (Archbishop) on Sep 19, 2001 at 17:33 UTC
    Try @openip = split(/\./, $stip); instead. '.' is a special character in regexes, meaning "Any character". You have to backslash it to make it work like a dot.

    On a sidenote, you should be doing the following, as well:

    1. Use strict and warnings. This will require you to use my on all your variables as you use them for the first time.
    2. Check the return value from open. The file may not be accessible, you may have mispelled the name, etc.

    ------
    We are the carpenters and bricklayers of the Information Age.

    Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

[reply]
[d/l]
      Everything dragonchild said holds true, let me append my .02 cents. This is how I would rewrite your script without altering too much. 
      #!/usr/bin/perl -w
# IP ranger

use strict;

my ($stip,$edip,$count,@openip);

open(IPFILE,"<iplist.txt") # open readonly
    || die "Could not open iplist.txt:$! \n";

while (<IPFILE>) {
    
    chomp; 
    
    ($stip,$edip) = split(/:/,$_,2); # split(/:/);

    print "$stip\n";
    print "$edip\n";

    @openip = split(/\./,$stip); 
    $count = @openip;

    print "$count\n"; 

    # you can avoid the creation of the $count
    # variable and use the following statement               
    # print scalar(@openip), "\n";
                

#    for ($i = 0; $i <= 4; $i++) {
#        print "@srtip\n";
#    };
};

close(IPFILE);

      [download]

      Update: semi-colon after open() was a poorly over looked error... fixed

      <font face="verdana" size=1"> - yo tengo mas que aprender
[reply]
[d/l]
        s0ttle has the rewrite correct. However, Perl idiom (and tradional coding practice) would dictate declaring a variable in tightest scope possible. Thus, I moved the my to within the while loop. 
        #!/usr/bin/perl -w
# IP ranger

# If you're on 5.6.0 or higher, add the following two lines
# and remove the -w

#use 5.6.0;
use strict;
#use warnings;

open(IPFILE,"<iplist.txt"); # open readonly
    || die "Could not open iplist.txt:$! \n";

while (<IPFILE>) {    
    chomp; 

    my ($stip,$edip) = split ':', $_, 2; # split(/:/);

    print "$stip\n";
    print "$edip\n";

    my @openip = split '.', $stip; # split(/\./,$stip);
    my $count = @openip;

    print "$count\n"; 

    # you can avoid the creation of the $count
    # variable and use the following statement               
    # print scalar(@openip), "\n";
                

#    for ($i = 0; $i <= 4; $i++) {
#        print "@srtip\n";
#    };
};

close(IPFILE);

        [download]

        ------
        We are the carpenters and bricklayers of the Information Age.

        Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

[reply]
[d/l]

        Watch the precedence of your ors! Compare the following lines of code: 

        my $foo = $bar || die "Bela Lugosi is dead: $!";
my $FOO = $BAR or die "I'm dead, I'm dead, I'm dead";

        [download]

        s0ttle's code looks pretty good (but I haven't run it), and dragonchild's point about moving variable declarations to get the smallest scope possible is good too. But their use of  open() || die makes me nervous.

        Remember that || has a very high precedence and or has a low precedence. In the code at the top of my post, line 1 dies if $bar evaluates to false; line 2 dies if the assignment to $FOO fails.

        I've been burned by using the || die syntax before. I kept doing it after some kindly monk (I forget who) warned me not to, and doomed myself to spend much time debugging as a result.


        Like Cassandra, TGI says moo

[reply]
[d/l]
[select]

        open(IPFILE,"<iplist.txt"); # open readonly
    || die "Could not open iplist.txt:$! \n";

        [download]
        will make the script fall over, due the extraneous semi-colon after the open call.

        @openip = split('.',$stip); # split(/\./,$stip); reproduces the original problem. That backslash needs to be there.

        What's the point of offering a rewritten version, if you're not even going to test it? Oh, XP whoring :-)

        I have to agree though, the use of strict, and moving chomp do make life easier in the long run.

        Just for the record, having fixed those two problems, I get: 

        10.1.2.2
10.1.2.200
4

[reply]
[d/l]
[select]
Re: beginner syntax question
by Anonymous Monk on Sep 19, 2001 at 19:38 UTC
    I think the problem is that you can't introduce a hash variable into a standard variable so perl just leaves $count = 0
[reply]
      $foo = @bar is a method for puting an element count into $foo, I like the suggestion to use the scalar command al la
       for ($i=0;$i <= scalar(@foo);$i) {print $foo[$i],"\n"}
      Forgetting to try escaping the . wasn't so bright. Thanks for the help guys
      Vec
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom