我的同事抱怨我的Perl看起来太像C了,这很自然,因为我大部分时间用C编程,而Perl只是一点点.这是我最近的努力.我对易于理解的Perl很感兴趣.我是一个Perl评论家,对于神秘的Perl几乎没有宽容.但考虑到可读性,下面的代码怎么能更多Perlish？
它的目标是进行流量分析,找出哪些IP地址在文件"ips"中给出的范围内.这是我的努力:
#!/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 () {
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 ( ) {
chomp;
if (defined $addresses{$_}) {
print "$_\n";
}
}
close (TRAFFIC);
}
Schwern.. 24
多年来看到C程序员编写的Perl代码,这里有一些通用的建议:
使用哈希值.使用列表.使用哈希!使用LISTS!使用列表操作(map,grep,split,join),尤其适用于小循环.不要使用花式列表算法; 流行,拼接,推,转移和不移动更便宜.不要使用树木; 哈希更便宜.哈希很便宜,制作它们,使用它们并把它们扔出去!使用迭代器for循环,而不是3-arg.不要调用$ var1,$ var2,$ var3; 改为使用列表.不要调用$ var_foo,$ var_bar,$ var_baz; 请改用哈希.使用$foo ||= "default".$_如果必须输入,请不要使用.
不要使用原型,这是一个陷阱!
使用正则表达式,而不是substr()或index().喜欢正则表达.使用/x修饰符使其可读.
statement if $foo在需要无块条件时写入.几乎总有一种更好的方法来编写嵌套条件:尝试递归,尝试循环,尝试散列.
在需要时声明变量,而不是在子程序的顶部.用严格.使用警告,并解决所有问题.使用诊断.写测试.写POD.
使用CPAN.使用CPAN!使用CPAN!有人可能已经做到了,更好.
运行perlcritic.运行它--brutal只是为了踢.运行perltidy.想想你为什么要做所有事情.改变你的风格.
使用不用于语言和调试内存分配的时间来改进代码.
问问题.慷慨地对您的代码进行风格评论.去参加Perl Mongers会议.转到perlmonks.org.去YAPC或Perl Workshop.您的Perl知识将实现跨越式发展.
多年来看到C程序员编写的Perl代码,这里有一些通用的建议:
使用哈希值.使用列表.使用哈希!使用LISTS!使用列表操作(map,grep,split,join),尤其适用于小循环.不要使用花式列表算法; 流行,拼接,推,转移和不移动更便宜.不要使用树木; 哈希更便宜.哈希很便宜,制作它们,使用它们并把它们扔出去!使用迭代器for循环,而不是3-arg.不要调用$ var1,$ var2,$ var3; 改为使用列表.不要调用$ var_foo,$ var_bar,$ var_baz; 请改用哈希.使用$foo ||= "default".$_如果必须输入,请不要使用.
不要使用原型,这是一个陷阱!
使用正则表达式,而不是substr()或index().喜欢正则表达.使用/x修饰符使其可读.
statement if $foo在需要无块条件时写入.几乎总有一种更好的方法来编写嵌套条件:尝试递归,尝试循环,尝试散列.
在需要时声明变量,而不是在子程序的顶部.用严格.使用警告,并解决所有问题.使用诊断.写测试.写POD.
使用CPAN.使用CPAN!使用CPAN!有人可能已经做到了,更好.
运行perlcritic.运行它--brutal只是为了踢.运行perltidy.想想你为什么要做所有事情.改变你的风格.
使用不用于语言和调试内存分配的时间来改进代码.
问问题.慷慨地对您的代码进行风格评论.去参加Perl Mongers会议.转到perlmonks.org.去YAPC或Perl Workshop.您的Perl知识将实现跨越式发展.
编写代码为"Perlish"的大多数都将利用Perl中的内置函数.
例如,这个:
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++);
}
}
}
我会改写像:
my @parts = split (/\./, $ip);
foreach my $part(reverse @parts){
$part++;
last unless ($part > 255 && !($part = 0));
}
这就是你上面发布的代码所做的,但更清洁一点.
你确定代码能做到你想要的吗？对我而言,如果后面的那个"大于255",你只会移动到IP的前一个"部分",这看起来有点奇怪.
有时,Perlish最常做的事情是转向CPAN而不是编写任何代码.
这是一个使用Net :: CIDR :: Lite和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();
}
免责声明:我刚刚说出来,它的测试很少,没有基准测试.省略完整性检查,错误处理和注释以保持行数减少.不过,我没有在这个空白处吝啬.
至于你的代码:在使用它们之前不需要定义你的函数.
另一个例子是重写:
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 ( ) {
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{$_};
}
正如前面提到的,尽量避免useING English除非你use English qw( -no_match_vars );避免那些邪恶的显著性能损失match_vars在那里.而且还没有被注意到,但应该......
ALWAYS ALWAYS ALWAYS总是use strict;和use warnings;否则Larry Wall的从天降临,打破你的代码.我知道你有-w- 这就足够了,因为即使在Unix之外,Perl也会解析shebang系列,并且会找到你想要的-w并且会use warnings;喜欢它.但是,你需要到use strict;.这将在代码中捕获许多严重错误,例如不使用my或使用false语言关键字声明变量.
让你的代码工作strict得很好,warnings将导致更清晰的代码永远不会因为你无法理解的原因而中断.您将花费数小时进行调试器调试,并且您可能最终会使用strict,warnings无论如何只是为了弄清楚错误是什么.只有当(并且仅当)您的代码完成并且您正在释放它并且它永远不会产生任何错误时,才删除它们.
虽然这样做肯定是Perl中的一种方法.
use strict;
use warnings;
my $new_ip;
{
my @parts = split ('\.', $ip);
foreach my $part(reverse @parts){
$part++;
if( $part > 255 ){
$part = 0;
next;
}else{
last;
}
}
$new_ip = join '.', reverse @parts;
}
这就是我实际实现它的方式.
use NetAddr::IP; my $new_ip = ''.(NetAddr::IP->new($ip,0) + 1) or die;
我不能说这个解决方案会让你的程序更加Perl-ish,但它可能会简化你的算法.
而不是将IP地址视为dot-quad,base-256数字,需要嵌套if结构来实现增量函数,而不是将IP地址视为32位整数.将abcd格式的IP转换为整数(未经测试):
sub ip2int {
my $ip = shift;
if ($ip =~ /(\d+)\.(\d+)\.(\d+)\.(\d+)/) {
return ($1 << 24) + ($2 << 16) + ($3 << 8) + $4;
} else {
return undef;
}
}
现在很容易确定IP是否落在两个端点IP之间.只需进行简单的整数运算和比较.
$begin = "192.168.5.0";
$end = "192.168.10.255";
$target = "192.168.6.2";
if (ip2int($target) >= ip2int($begin) && ip2int($target) <= ip2int($end)) {
print "$target is between $begin and $end\n";
} else {
print "$target is not in range\n";
}
告诉你的同事,他们的perl看起来太像线路噪音了.请不要仅仅为了混淆而混淆你的代码 - 它是asinine的开发目标,就像那些因为不可读而给人们带来如此糟糕的声誉,当它是非常糟糕的程序员(显然,就像你的同事)编写邋code的代码时.结构良好,缩进和逻辑代码是一件好事.C是件好事.
但是说真的 - 弄清楚如何编写perl的最佳位置是由Damian Conway撰写的O'Reilly"Perl Best Practices".它告诉你他怎么认为你应该做的事情,他总是给出他的立场的充分理由,偶尔给出不同意的充分理由.我在某些方面不同意他,但他的推理是合理的.你和任何比康威先生更了解perl的人合作的几率非常渺茫,并且拥有一本印刷书籍(或者至少是一个Safari订阅版)可以为你的论点提供更坚实的支持.拿着Perl Cookbook的副本,因为查看解决常见问题的代码示例可以让您走上正确的轨道.我不想说"买书",但这些都是非常好的书,任何 perl开发人员都应该阅读.
关于你的特定代码,你使用的是foreach,$_没有parens,shift,等等.它看起来很像perl-ish我的眼睛 - 已经用perl开发了很长一段时间.但需要注意的是 - 我讨厌英语模块.如果你必须使用它,那就这样吧use English qw( -no_match_vars );.match_vars选项可以显着降低regexp解析速度,并且它提供的$PREMATCH/ $POSTMATCH变量通常不常用.
京公网安备 11010802040832号 | 京ICP备19059560号-6 