Другой пример переписать:
sub is_less_than {
my $left = shift; # I'm sure you just "forgot" to put the my() here...
my $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
}
К этому:
sub is_less_than {
my @left = split(/\./, shift);
my @right = split(/\./, shift);
# one way to do it...
for(0 .. 3) {
if($left[$_] != $right[$_]) {
return $left[$_] < $right[$_];
}
}
# another way to do it - let's avoid so much indentation...
for(0 .. 3) {
return $left[$_] < $right[$_] if $left[$_] != $right[$_];
}
# yet another way to do it - classic Perl unreadable one-liner...
$left[$_] == $right[$_] or return $left[$_] < $right[$_] for 0 .. 3;
# just a note - that last one uses the short-circuit logic to condense
# the if() statement to one line, so the for() can be added on the end.
# Perl doesn't allow things like do_this() if(cond) for(0 .. 3); You
# can only postfix one conditional. This is a workaround. Always use
# 'and' or 'or' in these spots, because they have the lowest precedence.
return 0 == 1; # false is not a keyword, or a boolean value.
# though honestly, it wouldn't hurt to just return 0 or "" or undef()
}
Также здесь:
my ($ip, $end_ip, $junk) = split /,/;
$junk
может потребоваться @junk
для захвата всех мусора, или , вы, вероятно, можете его отключить - если вы назначите массив неизвестного размера для " массив "из двух элементов, он будет молча отбрасывать все лишние вещи. Так
my($ip, $end_ip) = split /,/;
А здесь:
foreach (@ARGV) {
open(TRAFFIC, $_) or die "Can't open $_ $OS_ERROR";
while (<TRAFFIC> ) {
chomp;
if (defined $addresses{$_}) {
print "$_\n";
}
}
close (TRAFFIC);
}
Вместо TRAFFIC
используйте переменную для хранения дескриптора файла. Кроме того, в общем, вы должны использовать exists()
, чтобы проверить, существует ли элемент хеша, а не defined()
- он может существовать, но быть установлен на undef
(это не должно происходить в вашей программе, но это хорошая привычка когда ваша программа усложняется):
foreach (@ARGV) {
open(my $traffic, $_) or die "Can't open $_ $OS_ERROR";
while (<$traffic> ) {
chomp;
print "$_\n" if exists $addresses{$_};
}
# $traffic goes out of scope, and implicitly closes
}
Конечно, вы также можете использовать замечательный оператор Perl <>
, который открывает каждый элемент @ARGV для чтения и действует как файловый дескриптор, который проходит через них:
while(<>) {
chomp;
print "$_\n" if exists $addresses{$_};
}
Как было отмечено ранее, старайтесь избегать use
ing English
, если вы не use English qw( -no_match_vars );
, чтобы избежать значительного штрафа за производительность тех злых match_vars
, находящихся там. И, как еще не было отмечено, но должно быть ...
ВСЕГДА ВСЕГДА ВСЕГДА всегда use strict;
и use warnings;
, иначе Ларри Уолл спустится с небес и нарушит ваш код. Я вижу, что у вас есть -w
- этого достаточно, потому что даже вне Unix Perl анализирует строку shebang и найдет ваш -w
и будет use warnings;
, как и должно быть. Тем не менее, вам нужно до use strict;
. Это вызовет много серьезных ошибок в вашем коде, например, не объявляя переменные с my
или используя false
в качестве ключевого слова языка.
Если ваш код работает под strict
, а также с warnings
, это приведет к НАМНОГО чистому коду, который никогда не сломается по причинам, которые вы не можете понять. Вы потратите часы на отладку отладчика и, возможно, в конечном итоге будете использовать strict
и warnings
, чтобы просто выяснить, в чем заключаются ошибки. Только удаляйте их, если (и только если) ваш код закончен и вы выпускаете его, и он никогда не генерирует никаких ошибок.