How can I write Perl that doesn't look like C?

Question

My co-workers complain that my Perl looks too much like C, which is natural since I program in C most of the time, and Perl just a bit. Here's my latest effort. I'm interest in Perl that is easy to understand. I'm a bit of a Perl critic, and have little tolerance for cryptic Perl. But with readability in mind, how could the following code be more Perlish?

It's goal is to do a traffic analysis and find which IP addresses are within the ranges given in the file "ips". Here's my effort:

#!/usr/bin/perl -w

# Process the files named in the arguments, which will contain lists of IP addresses, and see if 
# any of them are in the ranges spelled out in the local file "ip", which has contents of the
# form start-dotted-quad-ip-address,end-dotted-quad-ip_address,stuff_to_be_ignored
use English;


open(IPS,"ips") or die "Can't open 'ips' $OS_ERROR";

# Increment a dotted-quad ip address
# Ignore the fact that part1 could get erroneously large.
sub increment {
    $ip = shift;

    my ($part_1, $part_2, $part_3, $part_4) = split (/\./, $ip);
    $part_4++;
    if ( $part_4 > 255 ) {
        $part_4 = 0;
        ($part_3++);
        if ( $part_3 > 255 ) {
            $part_3 = 0;
            ($part_2++);
            if ( $part_2 > 255 ) {
                $part_2 = 0;
                ($part_1++);
            }
        }
   }   
    return ("$part_1.$part_2.$part_3.$part_4");
}

# Compare two dotted-quad ip addresses.
sub is_less_than {
    $left = shift;
    $right = shift;

    my ($left_part_1, $left_part_2, $left_part_3, $left_part_4)     = split (/\./, $left);
    my ($right_part_1, $right_part_2, $right_part_3, $right_part_4) = split (/\./, $right);


    if  ($left_part_1 != $right_part_1 ) { 
        return ($left_part_1 < $right_part_1);
    }   
    if  ($left_part_2 != $right_part_2 ) { 
        return ($left_part_2 < $right_part_2);
    }   
    if  ($left_part_3 != $right_part_3 ) { 
        return ($left_part_3 < $right_part_3);
    }
    if  ($left_part_4 != $right_part_4 ) {
        return ($left_part_4 < $right_part_4);
    }
    return (false);  # They're equal
}

my %addresses;
# Parse all the ip addresses and record them in a hash.   
while (<IPS>) {
    my ($ip, $end_ip, $junk) = split /,/;
    while (is_less_than($ip, $end_ip) ) {
        $addresses{$ip}=1;
        $ip = increment($ip);
    }
}

# print IP addresses in any of the found ranges

foreach (@ARGV) {
    open(TRAFFIC, $_) or die "Can't open $_ $OS_ERROR";
    while (<TRAFFIC> ) {
        chomp;
        if (defined $addresses{$_}) {
            print "$_\n";
        }
    }
    close (TRAFFIC);

}

Answer 1

Sometimes the most Perlish thing to do is to turn to CPAN instead of writing any code at all.

Here is a quick and dirty example using Net::CIDR::Lite and Net::IP::Match::Regexp:

#!/path/to/perl

use strict;
use warnings;

use English;
use IO::File;
use Net::CIDR::Lite;
use Net::IP::Match::Regexp qw(create_iprange_regexp match_ip);


my $cidr = Net::CIDR::Lite->new();

my $ips_fh = IO::File->new();

$ips_fh->open("ips") or die "Can't open 'ips': $OS_ERROR";

while (my $line = <$ips_fh>) {

    chomp $line;

    my ($start, $end) = split /,/, $line;

    my $range = join('-', $start, $end);

    $cidr->add_range($range);

}

$ips_fh->close();

my $regexp = create_iprange_regexp($cidr->list());

foreach my $traffic_fn (@ARGV) {

    my $traffic_fh = IO::File->new();

    $traffic_fh->open($traffic_fn) or die "Can't open '$traffic_fh': $OS_ERROR";

    while (my $ip_address = <$traffic_fh>) {

        chomp $ip_address;

        if (match_ip($ip_address, $regexp)) {
            print $ip_address, "\n";
        }     

    }

    $traffic_fh->close();

}

DISCLAIMER: I just banged that out, it's had minimal testing and no benchmarking. Sanity checks, error handling and comments omitted to keep the line count down. I didn't scrimp on the whitespace, though.

As for your code: There is no need to define your functions before you use them.