Perl -代码审查

Question

我正在开发一个程序，该程序将CSV文件中的信息作为源，通过具有“客户包”的文本文件进行搜索。我只在一些条目上得到了奇怪的计数，而且我似乎找不出是什么导致了重复计数。有没有人能看一眼我的代码，告诉我我的逻辑/语法有没有问题？(可能是)。我所要做的就是统计csv文件中一个条目(packageid，package_description)在文本文件中的总出现次数。

谢谢你的帮助！我在这里发疯了。

#!/usr/bin/perl

use strict;
use Text::CSV;

# Variables already declared in the other PL file ** Remove if consolidating **

my $file2 = 'master_plist.csv';
my $csv2 = Text::CSV->new(); # Create a Text::CSV object

open (CSV2, "<", $file2) or die $!; #open CSV file for parsing

while (<CSV2>) {

    if ($csv2->parse($_)) {
            my @columns2 = $csv2->fields(); # Parse CSV and load into an array for each row.
            my $packID = $columns2[0];
            my $packDESC = $columns2[1];



my $val = 'customer_packages_report.txt';

chomp ($val);

my $cnt=0;

open (HNDL, "$val") || die "wrong filename";

while ($val = <HNDL>)
{
while ($val =~ /$packID - $packDESC/ig)
{
    $cnt++;
}
}

#if ($packDESC =~ /\(/g) {
#       $packDESC =~ s/\(/\(/g;
#} 
print "Total iterations of $packDESC: $cnt\n";

close (HNDL);
# End original code

    } # Close IF
} # Close WHILE

close CSV;

Answer 1

#!/usr/bin/perl

use strict;
use warnings;
use Text::CSV;

# Variables already declared in the other PL file ** Remove if consolidating **

my $file2 = 'master_plist.csv';
my $csv2 = Text::CSV->new(); # Create a Text::CSV object

open (CSV2, "<", $file2) or die "I die while opening $file2!  $!"; #open CSV file for parsing

while ($each_csv2_line=<CSV2>) {

    if ($csv2->parse($each_csv2_line)) {
            my @columns2 = $csv2->fields(); # Parse CSV and load into an array for each row.
            my $packID = $columns2[0];
            my $packDESC = $columns2[1];



            my $val = 'customer_packages_report.txt';

            chomp ($val);

            my $cnt=0;

            open (HNDL,"<","$val") or die "wrong filename: $val! $!";

            while (<HNDL>){
                $cnt++ while (/$packID - $packDESC/ig);
            }

#if ($packDESC =~ /\(/g) {
#       $packDESC =~ s/\(/\(/g;
#} 
            print "Total iterations of $packDESC: $cnt\n";

            close (HNDL);
            # End original code

    } # Close IF
} # Close WHILE

# end of script
close CSV;

我的建议是：

• 使用$HNDL instead of HNDL <- lexical变量作为文件句柄更好，
• 尝试捕获所有错误(由defined和==0编写)，==0尝试格式化您的代码并添加一些我有时会用到的功能。比我强，先读Style Coding for Little Perl Monk。您不仅可以编写writeonly代码，还可以更好地使用这种语言。

示例(还有一个引号)：

“行输入操作符<>的情况与此完全相同，尽管Perl会自动为您执行此操作。

看起来您正在测试来自STDIN的代码行：

    while (<STDIN>) {
       do_something($_);
    }

但是，这是一种特殊情况，在这种情况下，Perl会自动转换为检查$_的定义：

     while ( defined( $_ = <STDIN> ) ) {  # implicitly done
       do_something($_);
     }

“有效的Perl编程”，第24页。

Answer 2

你可以做很多事情来改进你的代码：

1. use warnings;.
2. Use正确的indentation.
3. Use描述性变量名。代替$file2 (没有意义，为什么没有文件1?)，使用$package_file或任何有意义的东西。
4. 如果你已经在使用Text::CSV，你可以使用$csv->getline()逐行遍历文件。这将简化您的代码。See the documentation for an example.
5. chomp($val)从字符串的末尾删除换行符。您正在对您刚刚声明的字符串文字使用它，该字符串没有换行符。这并不会使sense.
6. Never使用相同的变量($val)来做两件完全不同的事情。这是非常confusing.
7. Might的，你在正则表达式中插值的变量包含特殊字符？如果是这样的话，你需要逃离它们。例如，如果$packDESC包含一个句点，它将匹配正则表达式中的任何字符。要逐字处理变量的内容，请使用\Q..\E，如下例所示：/\Q$packID - $packDESC\E/ig.
8. You打开customer_packages_report.txt，并在csv文件的每一行逐行遍历它。你可以通过一次读取它并将结果存储在一个数组中来简化这一过程。
9. 你不需要一个while循环来计算matches：$cnt = () = /$packID - $packDESC/ig;。这会将匹配放入数组上下文中，返回匹配数组，然后将其放回标量上下文以计算匹配数。有点棘手，但很简单。

在看不到数据的情况下，很难准确地说出是什么导致了您的问题。你会不会有一些不必要的重复，这源于你在两个文件上的嵌套循环？我会从重写来改进你的代码开始，然后看看问题是否仍然存在。

Answer 3

您的代码似乎可以用perl -c编译而没有错误，所以这很好。如果我要猜测，我会假设您的问题出在您的某些字段中包含元字符。正则表达式/$packID - $packDESC/容易受到元字符的攻击。例如

my $str = "foo? bar";
$str =~ /$str/;       # returns false, because ? is a meta character

在上面的示例中，问号?是一个量词，它影响它之前的任何内容，因此o?表示"0或1 o“。要解决元字符问题，请使用\Q ... \E转义：

$str =~ /\Q$str/;   # will now match

使用\E终止转义序列是可选的。

其他一些需要注意的事情：

• 你使用use strict是非常好的。您还应该始终使用use warnings。不这样做并不能消除代码中的问题，而只是隐藏问题。
• 您可以使用默认设置创建Text::CSV对象。根据您的输入，这可能是合适的，也可能不合适。建议在the documentation.
• Using中设置binary => 1 parse()函数可能不是最好的选择，文档中有关于getline.
• As的好东西要说在注释中指出，您正在重用$val来读取您的文件。虽然从技术上讲，这应该是可行的，但它是在自找麻烦。

风格、练习笔记和实用提示：

• 使用三参数打开和词法文件句柄是一件很好的事情。三参数本质上意味着使用显式的开放模式，这使得您的脚本使用起来更安全。使用词法文件句柄意味着在文件句柄上不会有全局作用域，这是一件好事。
• This code

my @columns2 = $csv2->fields(); 
my $packID = $columns2[0];
my $packDESC = $columns2[1];

可以写成这样

my ($packID, $packDESC) = $csv2->fields();

• 在你赋值之后，你正在吞噬$val。这是多余的，因为chomp默认情况下只删除字符串末尾的换行符，而您并没有添加任何这样的行。它不会改变任何东西，但在这里不是必需的。如果你从标准输入或文件中读取一些东西，你可能会想要使用chomp，though.
• Using die而不引用错误$!肯定会让你自己感到恼火。
• 不要低估了当你使用适当的缩进时，编写代码会变得多么容易。使用具有自动缩进和着色功能的文本编辑器。我可以热情地推荐vim (如果你使用的是windows的话就是gvim)。虽然它有一个学习曲线，但它是一个功能强大的编辑器，通常也已经安装在许多系统上。