程序修复店和红旗
这张图片有什么问题？ 界面 代码 - open_info_file - start_info_file - start_next_part - read_next_node - 寻找菜单
整合一切 红旗 - 不要重复代码 - eof() - return 0 and return undef
这张图片有什么问题？
我再次将审视一个Perl初学者的程序，看看我能做些什么来改进它。
这个月的程序来自一个非常古老的Usenet帖子。它是在七年前的1993年11月12日发布的，确切地说，是在comp.lang.perl新闻组上。（那时comp.lang.perl.misc尚未创建。）
这个程序是一个用于读取GNU“info”文件的代码库。Info文件是GNU项目使用的结构化文档的一种形式。如果您使用emacs编辑器，可以通过使用C-h i命令等来浏览info文件。info文件由许多节点组成，每个节点包含有关某个主题的信息。节点以树状结构排列。每个节点都有一个带有一些元信息的标题；记录在每个节点标题中的一个项目是该节点在文档树中的父节点名称。大多数节点还具有其子节点的菜单。每个节点还具有指向下一个和上一个节点的指针，以便您可以按顺序阅读所有节点。
界面
我们将看到的代码有打开info文件和读取节点以及解析它们标题和菜单中的信息的函数。但在我开始讨论代码之前，我会展示文档。以下就是它，直接从那个7年前的Usenet帖子复制而来，包括所有的错误
To use the functions: Call
&open_info_file(INFO_FILENAME);
to open the filehandle `INFO' to the named info file.
Then call
&get_next_node;
repeatedly to read the next node in the info file; variables
$info_file
$info_node
$info_prev
$info_next
$info_up
are set if the corresponding fields appear in the node's
header, and if the node has a menu, it is loaded into
%info_menu. When `get_next-node' returns false, you have
reached end-of-file or there has been an error.
立即就可以看出一个主要问题。代码应该是一个实用函数库。但库和主程序之间唯一的通信是通过一系列具有如$info_up等名称的全局变量。这当然是非常糟糕的风格。函数不能在恰好有一个名为$info_up的变量的程序中使用，如果您在这样一个程序中使用它，可能会引入一些奇特、难以发现的错误，这些错误是由库破坏该变量之前值的某种方式引起的。库甚至可能干扰自身！如果您有像这样的事情
&get_next_node;
foo();
print $info_node;
那么您可能不会得到预期的结果。如果foo()恰好也调用get_next_node，它将丢弃主代码计划打印的$info_node的值。
这些问题正是函数和局部变量打算解决的问题。在这种情况下，很容易解决问题：让get_next_node返回节点信息的列表，而不是设置一些硬编码的全局变量。如果函数的调用者想要自己设置变量，它仍然可以这样做
%next_node = &get_next_node;
($info_file, $info_node, $info_prev, $info_next, $info_up)
= @next_node{qw(File Node Prev Next Up)};
%info_menu = %{$next_node{Menu}}
或者不这样做
my (%node) = &get_next_node;
my ($next) = $node{Next};
如果出于某种原因，调用者喜欢get_next_node的全局变量，他们仍然可以保留原始接口
sub get_next_node_orig {
my %next_node = &get_next_node;
($info_file, $info_node, $info_prev, $info_next, $info_up)
= @next_node{qw(File Node Prev Next Up)};
%info_menu = %{$next_node{Menu}}
}
这表明没有丢失任何功能；无论是返回值列表还是直接设置全局变量，其功能都是相同的。
代码
现在我们将看到代码本身。 整个程序在这里可以找到。我们将一次查看一个部分。
open_info_file
用户首先调用的函数是 open_info_file 函数
83 sub open_info_file {
84 ($info_filename) = @_;
85 (open(INFO, "$info_filename"))
|| die "Couldn't open $info_filename: $!";
86 return &start_info_file;
87 }
在讨论这里的架构问题之前，有一个小的语法问题：围绕 "$info_filename" 的引号是没有用的。Perl 使用 "..." 语法来表示“构造一个字符串”。但是 $info_filename 已经是一个字符串了，因此将其转换为字符串最多是浪费时间。此外，多余的引号有时会导致微妙的错误。考虑以下看似无害的代码
my ($x) = @_;
do_something("$x");
如果 $x 是一个字符串，这仍然可以工作。但如果 $x 是一个 引用，它可能就会失败。为什么？因为 "$x" 构造了一个看起来像引用但实际上不是的字符串，如果 do_something 期望一个引用，它将会失望。这样的错误很难调试，因为当你打印出来时，do_something 收到的字符串看起来像是一个引用。use strict 'refs' 预言就是为了捕捉这种错误。在有 use strict 'refs' 的范围内，do_something 可能会抛出一个类似以下的错误
Can't use string ("SCALAR(0x8149bbc)") as an ARRAY ref...
没有 use strict 'refs'，你会得到一个微妙且无声的bug。
但是回到代码。如果由于任何原因无法打开指定的文件，open_info_file 会调用 die。在这种情况下，让它简单地返回一个失败码可能更方便、更一致；如果 open 成功，然后 start_next_part 失败，它就是这样做的。对于调用代码来说，处理简单的错误返回通常比处理异常更容易，尤其是在1993年，那时Perl还没有异常处理。我会这样重写这个函数
sub open_info_file {
($info_filename) = @_;
open(INFO, $info_filename) || return;
return start_info_file();
}
我还去掉了多余的括号，并将1993年的 &function 语法改为更现代的 function() 语法。试图将 $info_filename 变成一个私有变量很有诱惑力，但结果发现其他函数后来也需要看到它，所以我们能做的最好的事情是将它变成一个文件作用域的词法，私有于库，但由库中的所有函数共享。
最后，一个设计问题：文件句柄名 INFO 被硬编码到函数中。由于文件句柄名是全局变量，所以我们最好避免它，原因和我们要在前面的代码中去除 $info_node 变量的原因相同：如果程序的另一部分恰好有一个名为 INFO 的文件句柄，它将会非常惊讶地发现它突然被附加到一个新的文件上。
有几种方法可以解决这个问题。在Perl 4中可用的是最好的方法之一，即让调用者通过将文件句柄作为 open_info_file 的参数传入来传递它。然后调用实际上是在使用文件句柄作为对象。然而，在这种情况下，这并不像我们希望的那样好，因为，正如我们稍后会看到的，库需要能够将文件名与文件句柄关联起来。在原始库中，这很容易，因为文件名总是存储在全局变量 $info_filename 中，而文件句柄总是 INFO。这个简单解决方案的缺点是库不能同时打开两个info文件。在Perl 4中有解决这个问题的方法，但对Perl 4程序员来说才有兴趣，所以我不将详细说明。
在Perl 5中，解决方案是使用一个对象来表示一个打开的信息文件。每当调用者想要对文件进行操作时，它将对象作为参数传递给库。对象可以携带打开的文件句柄和文件名。由于对象内部的数据是私有的，所以它不会干扰程序中的任何其他数据。调用者可以同时打开多个文件，并且可以通过它们各自的对象来区分它们。
要将这个库转换为一个面向对象类，只需要进行几个小的改动。我们在顶部添加
package Info_File;
，并将open_info_file重写如下
sub open_info_file {
my ($class, $info_filename) = @_;
my $fh = new FileHandle;
open($fh, $info_filename) || return;
my $object = { FH => $fh, NAME => $info_filename };
bless $object => $class;
return unless $object->start_info_file;
return $object;
}
我们现在可以这样调用函数
$object = Info_File->new('camel.info');
new FileHandle这一行创建了一个全新的文件句柄。下一行按照惯例打开文件句柄。这一行
my $object = { FH => $fh, NAME => $info_filename };
构建了一个对象，它只是一个散列。对象包含了库处理信息文件所需的所有信息 - 在这种情况下，打开的文件句柄和原始文件名。《code>bless函数将散列转换为Info_File类的完整对象。最后，
$object->start_info_file;
以$object作为参数调用start_info_file函数，就像调用start_info_file($object)一样。这种特殊的“箭头”语法是由上一行的bless启用的。这种表示法表示对象上的方法调用；start_info_file是方法。方法只是一个普通的子程序。对象上的方法调用就像任何其他子程序调用一样，只是子程序将对象本身作为参数传递。
在一个三行函数上花费这么多空间似乎有些浪费，但很多相同的问题会反复出现，在一个简单的上下文中看到它们是很有好处的。
start_info_file
47 # Discard commentary before first node of info file
48 sub start_info_file {
49 $_ = <INFO> until (/^\037/ || eof(INFO));
50 return &start_next_part if (eof(INFO)) ;
51 return 1;
52 }
信息文件通常在第一个节点之前有一个序言，通常包含版权声明和许可证。当用户打开信息文件时，库需要跳过这个序言以到达节点，这些是感兴趣的部分。这就是start_info_file的作用。序言与第一个节点之间由以神秘的\037字符开始的行分开，这是控制下划线。该函数将逐行读取文件，寻找以神秘字符开始的第一个行。如果找到这样的行，它将立即返回成功。否则，它将移动到下一个“部分”，我稍后会解释。
正如我在前面的文章中解释的那样，“红旗”是一个立即的警告信号，表明你做错了某事。使用eof()函数是Perl中最清晰、最显眼的红旗之一。几乎总是使用eof()是错误的。
eof()的问题在于它试图看到未来，即从文件句柄的下一次读取将返回文件结束条件。实际上看到未来是不可能的，所以它实际上尝试读取一些数据。如果没有数据，它将报告下一次读取也将报告文件结束。如果没有，它必须将刚刚读取的数据放回。这可能导致奇怪的问题，因为eof()正在读取你可能没有打算读取的额外数据。
eof()是那些看起来很有用，但后来发现几乎总是有更好的方法来实现相同功能的功能之一，就像goto一样。在这种情况下，代码更加直接和规范，如下所示
sub start_info_file {
while (<INFO>) {
return 1 if /^\037/;
}
&start_next_part;
}
Perl会在到达文件末尾时自动退出while循环，在这种情况下，我们可以无条件地调用start_next_part。在循环内部，我们检查当前行是否是分隔符，如果是，就返回成功。对$_的赋值和对文件结束的检查现在是隐式的。
在面向对象风格中，start_info_file期望从其参数中获取一个对象，该对象最初由open_info_file构造，它将包含该函数将要从中读取的文件句柄，而不是使用INFO。将其重写为面向对象风格很简单
sub start_info_file {
my ($object) = @_;
my $fh = $object->{FH};
while (<$fh>) {
return 1 if /^\037/;
}
$object->start_next_part;
}
在这里，我们通过请求$object->{$fh}从对象中提取文件句柄，然后使用文件句柄$fh代替INFO。对start_next_part的调用变为对对象的函数调用，这意味着对象被隐式地传递给start_next_part函数，这样start_next_part也能访问到对象，包括其中埋藏的文件句柄。
start_next_part
我承诺要解释start_next_part的作用，现在我们到了这里。信息文件不是单个文件；它可能被分成几个单独的文件，每个文件包含一些节点。如果主信息文件命名为camel.info，那么可能还有额外的节点在camel.info-1、camel.info-2等文件中。这意味着当我们到达信息文件的末尾时，我们还没有完成；我们必须检查它是否在另一个文件中继续。start_next_part就是做这个的。
54 # Look for next part of multi-part info file. Return 0
55 # (normal failure) if it isn't there---that just means
56 # we ran out of parts. die on some other kind of failure.
57 sub start_next_part {
58 local($path, $basename, $ext);
59 if ($info_filename =~ /\//) {
60 ($path, $basename)
= ( $info_filename =~ /^(.*)\/(.*)$/ );
61 } else {
62 $basename = $info_filename;
63 $path = "";
64 }
65 if ($basename =~ /-\d*$/) {
66 ($basename, $ext)
= ($basename =~ /^([^-]*)-(\d*)$/);
67 } else {
68 $ext = 0;
69 }
70 $ext++;
71 $info_filename = "$path/$basename-$ext";
72 close(INFO);
73 if (! (open(INFO, "$info_filename")) ) {
74 if ($! eq "No such file or directory") {
75 return 0;
76 } else {
77 die "Couldn't open $info_filename: $!";
78 }
79 }
80 return &start_info_file;
81 }
此代码的主要目的是将类似于/usr/info/camel.info-3的文件名更改为/usr/info/camel.info-4。它必须处理一个特殊情况：/usr/info/camel.info必须变为/usr/info/camel.info-1。在计算新的文件名后，它尝试打开信息文件的下一部分。如果成功，它将调用start_info_file跳过新部分的序言。
首先要注意的是，该函数正在做比它需要的更多的工作。它仔细地将文件名分离成目录名和基本名，通常是/usr/info和camel.info-3。但这一步是不必要的，让我们消除它。
sub start_next_part {
local($name, $ext);
if ($info_filename =~ /-\d*$/) {
($name, $ext)
= ($info_filename =~ /^([^-]*)-(\d*)$/);
} else {
$ext = 0;
}
$ext++;
$info_filename = "$name-$ext";
# ... no more changes ...
}
这立即将函数的大小减少了25%。现在我们注意到剩下的两个模式匹配几乎相同。这是所有红旗的红旗：任何程序做两件事的时候，看看是否可以只做一次。有时你做不到。这一次，我们可以
sub start_next_part {
local($name, $ext);
if ($info_filename =~ /^([^-]*)-(\d*)$/) {
($name, $ext) = ($1, $2);
} else {
$name = $info_filename; $ext = 0;
}
$ext++;
$info_filename = "$name-$ext";
# ... no more changes ...
}
这要简单一些，并为一个大的改进铺平了道路：$name变量是多余的，因为它的唯一目的就是保存一个中间结果。真正感兴趣的变量是$info_filename。$name是我所说的合成变量：它是我们解决问题的方式的产物，对于问题本身来说是不必要的。在这种情况下，很容易消除它
sub start_next_part {
if ($info_filename =~ /^([^-]*)-(\d*)$/) {
$info_filename = $1 . '-' . ($2 + 1);
} else {
$info_filename .= '-1';
}
# ... no more changes ...
}
如果模式匹配，那么$1包含基本名，通常是/usr/info/camel.info，而$2包含数字后缀，通常是3。在使用它们之前没有必要将它们复制到命名变量中；我们可以直接从$1和$2中构造新的文件名，即/usr/info/camel.info-4。如果模式不匹配，我们通过在旧文件名后追加-1来构造新的文件名；这把/usr/info/camel.info变成/usr/info/camel.info-1。
这样就处理了函数的上半部分；现在让我们看看下半部分
sub start_next_part {
if ($info_filename =~ /^([^-]*)-(\d*)$/) {
$info_filename = $1 . '-' . ($2 + 1);
} else {
$info_filename .= '-1';
}
close(INFO);
if (! (open(INFO, "$info_filename")) ) {
if ($! eq "No such file or directory") {
return 0;
} else {
die "Couldn't open $info_filename: $!";
}
}
return &start_info_file;
}
close(INFO)是不必要的，因为下一行的open将执行隐式关闭。如果文件不能打开，函数会查找原因。如果原因是下一部分不存在，那么我们真的到了结尾，它将安静地返回失败，但如果是有其他类型的错误，它将终止。按照我们对open_info_file的更改，我们将消除die，如果需要，让调用者自己终止
sub start_next_part {
if ($info_filename =~ /^([^-]*)-(\d*)$/) {
$info_filename = $1 . '-' . ($2 + 1);
} else {
$info_filename .= '-1';
}
return unless open(INFO, $info_filename);
return &start_info_file;
}
我在这里进行了一些小的修改：去掉了围绕$info_filename的冗余引号，将if !改为unless。我还将return 0替换为return。 return 0和return undef是红旗：它们是尝试创建一个返回假值的函数。但如果是函数在列表上下文中调用，那么0和undef的返回值被解释为真，而不是假，因为它们是单元素列表，而唯一的假列表是空列表。
sub false {
return 0;
}
@a = false();
if (@a) {
print "ooops!\n";
}
在Perl中，函数返回布尔值通常是一个简单的return，就像我们在这里所做的那样。在标量上下文中，这返回一个未定义的值；在列表上下文中，它返回一个空列表。
函数从20行缩减到7行。将其重新调整以适应面向对象风格并没有让它变得更大。
sub start_next_part {
my ($object) = @_;
my $info_filename = $object->{NAME};
if ($info_filename =~ /^([^-]*)-(\d*)$/) {
$info_filename = $1 . '-' . ($2 + 1);
} else {
$info_filename .= '-1';
}
my $fh = $object->{FH};
return unless open($fh, $info_filename);
$object->{NAME} = $info_filename; # ***
return $object->start_info_file;
}
在这里，我们使用$object->{NAME}从对象中提取信息文件的文件名，这是我们最初在open_info_file中设置的。我们还使用$object->{FH}从对象中提取文件句柄，就像我们在start_info_file中做的那样。如果我们成功打开新文件，我们将更改后的文件名存储回对象中，以备下次使用；这发生在标记为***的行。
read_next_node
最后，我们来到了库的核心。实际上，read_next_node读取一节信息并将其返回给调用者。（首先要注意的是，文档将此函数称为get_next_node，这是错误的。但这很容易修复。）
从本函数的角度来看，节点有三个部分。第一行是节点的标题，其中包含节点的名称；指向先前和下一个节点的指针；以及其他元信息。然后是一段很长的文本，这是节点打算包含的文档。在文本的底部附近有一个指向其他节点的指针菜单。read_next_node对标题行和菜单感兴趣。它有三个部分：一个部分用于处理标题行，一个部分跳过随后的文本直到看到菜单，一个部分用于解析菜单。我们将逐一处理这些问题。
1 # Read next node into global variables. Assumes that file
2 # pointer is positioned at the header line that starts a
3 # node. Leaves file pointer positioned at header line of
4 # next node. Programmer: note that nodes are separated by
5 # a "\n\037\n" sequence. Reutrn true on
success, false on failure
6 sub read_next_node {
7 undef %info_menu;
8 $_ = <INFO>; # Header line
9 if (eof(INFO)) {
10 return &start_next_part && &read_next_node;
11 }
12
13 ($info_file) = /File:\s*([^,]*)/;
14 ($info_node) = /Node:\s*([^,]*)/;
15 ($info_prev) = /Prev:\s*([^,]*)/;
16 ($info_next) = /Next:\s*([^,]*)/;
17 ($info_up) = /Up:\s*([^,]*)/;
这里不需要做太多改变。当%info_menu是一个全局变量时，undef %info_menu是一个适当的初始化，但我们的函数不会使用全局变量；它将返回列表作为其返回列表的一部分，因此我们将此行替换为my %info_menu。eof()测试再次是一个红旗；简单地检查$_是否定义可能更直接。如果它未定义，则函数已到达文件末尾，需要尝试打开下一部分。如果这成功了，那么它将递归调用自己来读取新部分的第一节点。这里用来序列这两个操作的&&简洁而略带奇特。不幸的是，现在read_next_node返回一个数据列表，它将不再工作，因为&&始终在标量上下文中评估其参数。这部分代码需要更改为：
$_ = <INFO>; # Header line
if (! defined $_) {
return unless &start_next_part;
return &read_next_node;
}
递归调用可能看起来有点奇怪，因为它本质上是在函数顶部执行一个goto回跳，有些人可能会用简单的while循环来表示这一点。但这样做是否更清晰并不明显，所以我决定保留递归调用。
随后的行将标题的部分提取到全局变量$info_file、$info_node等中。由于我们需要将这些项制作成一个要从函数返回的数据结构，而不是一组全局变量，因此尝试这样做是自然的：
($header{File}) = /File:\s*([^,]*)/;
($header{Node}) = /Node:\s*([^,]*)/;
($header{Prev}) = /Prev:\s*([^,]*)/;
($header{Next}) = /Next:\s*([^,]*)/;
($header{Up}) = /Up:\s*([^,]*)/;
这有效，但正如我之前提到的，重复的代码是最大的红旗之一。这五行的相似性表明我们应该尝试使用循环：for my $label (qw(File Node Prev Next Up)) { ($header{$label}) = /$label:\s([^,])/; }
这五行变成了两行。然而，缺点是Perl必须为每个节点重新编译五次模式，因为$label的值不断变化。我们可以做三件事来处理这个问题。我们可以忽略它，我们可以应用qr//运算符来预编译模式，或者我们可以尝试将五个变量模式合并成一个常量模式。在我看来，对于大多数微优化问题，如果它不证明是一个真正的问题，我们应该忽略它。在这种情况下，qr//解决方案将是一个充分的替代方案。
我还考虑将它们合并为一个模式，但这变成了一个灾难
($file, $node, $prev, $next, $up) =
/File:\s*([^,]*),\s*Node:\s*([^,]*),\s*
Next:\s*([^,]*),\s*Prev:\s*([^,]*),\s*
Up:\s*([^,]*)/x;
实际上，情况比这更糟，因为五个项目中的某些可能从标题行中缺失，因此我们必须使每个部分都是可选的
($file, $node, $prev, $next, $up) =
/(?:File:\s*([^,]*),)?\s*(?:Node:\s*([^,]*),)?\s*
(?:Next:\s*([^,]*),)?\s*(?:Prev:\s*([^,]*),)?\s*
(?:Up:\s*([^,]*))?/x;
实际上，情况甚至更糟，因为原始作者是用Perl 4编写的，没有(?:...)或/x。所以这个策略根本不起作用。
这提出了一个重要观点，我并不总是像应该那样强调：直到你尝试过，你并不总是清楚哪些策略是最佳的。当我写这些文章时，我会犯错误。我以一种方式重写代码，发现存在意想不到的问题，而且收益并不像我之前想象的那么大。然后，我尝试另一种方式，看看是否看起来更好。有时我发现自己错了，原始代码赢了，就像在这个例子中一样。
当你自己编写代码时，你并不总是清楚如何最好地继续下去。尝试两种方式，看看哪一种看起来更好，然后扔掉你不喜欢的那一种。
在这篇文章中，我最初计划将库重构成在Perl 4下仍然可以工作的一种形式。我写了很多文字来解释如何做到这一点。但结果是，唯一好的解决方案是对象，所以我重新做了，这就是你看到的样子。
道德：永远不要害怕重做。
好吧，结束离题。函数已经处理了标题行；现在它需要跳过中间的文本，直到找到节点中的菜单部分
19 $_ = <INFO> until /^(\* Menu:|\037)/ || eof(INFO);
20 if (eof(INFO)) {
21 return &start_next_part;
22 } elsif (/^\037/) {
23 return 1; # end of node, so return success.
24 }
菜单跟在标注为* Menu:的行后面。如果函数在看到* Menu之前看到了节点的末尾或文件的末尾，那么这个节点没有菜单。这里有一个错误：函数应该在节点末尾立即返回，无论它是否也是文件的末尾。最初编写的代码在文件末尾调用start_next_part，这可能失败（如果当前节点是最后一个），在应该报告成功时报告失败。修复错误并消除eof()得到这个
$_ = <INFO> until !defined($_) || /^(\* Menu:|\037)/;
return @header if !defined($_) || /^\037/;
那里重复的测试让我感到困扰，但我能想到的最佳替代方案是
while (<INFO>) {
last if /^\* Menu:/;
return %header if /^\037/;
}
return %header unless defined $_;
我四处询问，Simon Cozens建议
do {
$_ = <INFO>;
return %header if /^\037/ || ! defined $_
} until /^\* Menu:/ ;
我认为这是我最喜欢的，因为它将/^\* Menu:变成了主要终止条件，这是应该的。另一方面，do...until是不常见的，而且你得不到对$_的隐式读取。但是四种相同的代码已经足够多了，所以让我们继续前进。
最后，我们的函数准备好读取菜单了。一个典型的菜单看起来像这样
* Menu:
* Numerical types::
* Exactness::
* Implementation restrictions::
* Syntax of numerical constants::
* Numerical operations::
* Numerical input and output::
每个条目都有一个标题（这是显示给用户的）和一个节点名称（如果用户选择该菜单项，则用户将访问的节点）。如果标题和节点名称不同，菜单项看起来像这样
* The title: The node name.
如果它们相同（这种情况很常见），菜单项以::结尾，就像上面的例子一样。菜单读取代码必须处理这两种情况
27 local($key, $ref);
28 while (<INFO>) {
29 return 1 if /^\037/; # end of node, success.
30 next unless /^\* \S/; # skip non-menu-items
31 if (/^\* ([^:]*)::/) { # menu item ends with ::
32 $key = $ref = $1;
33 } elsif (/^\* ([^:]*):\s*([^.]*)[.]/) {
34 ($key, $ref) = ($1, $2);
35 } else {
36 print STDERR "Couldn't parse menu item\n\t$_";
37 next;
38 }
39 $info_menu{$key} = $ref;
40 }
我认为这段代码很棒。但我只会做两处改动。首先，我会将错误信息改为包含格式不正确的菜单条目所在的文件名和行号。Perl 内置的 $. 变量使得这变得很容易，而当前的行为使得程序员难以找到问题的根源。其次，而不是直接从循环中 return，我会使用 last，因为返回值 (%header, Menu => \%menu) 相对复杂，而循环下面的代码也必须返回同样的值。
在原始程序中，如果函数在读取菜单的同时读取到当前部分的末尾，那条 return 语句会再次调用 start_info_file。这是不正确的；它应该简单地返回成功，让下一次对 read_next_node 的调用去处理打开新部分。
重写的 read_next_node 看起来是这样的
sub read_next_node {
$_ = <INFO>; # Header line
if (! defined $_) {
return unless &start_next_part;
return &read_next_node;
}
my (%header, %menu);
for my $label (qw(File Node Prev Next Up)) {
($header{$label}) = /$label:\s*([^,]*)/;
}
do {
$_ = <INFO>;
return %header if /^\037/ || ! defined $_
} until /^\* Menu:/ ;
while (<INFO>) {
my ($key, $ref);
last if /^\037/; # end of node
next unless /^\* \S/; # skip non-menu-items
if (/^\* ([^:]*)::/) { # menu item ends with ::
$key = $ref = $1;
} elsif (/^\* ([^:]*):\s*([^.]*)[.]/) {
($key, $ref) = ($1, $2);
} else {
warn "Couldn't parse menu item at line $.
of file $info_file_name";
next;
}
$menu{$key} = $ref;
}
return (%header, Menu => \%menu);
}
这次代码没有变得更短，因为这开始就很好。经过一些简单的改动，将其转换为面向对象风格后，我们得到
sub read_next_node {
my ($object) = @_;
my ($fh) = $object->{FH};
local $_ = <$fh>; # Header line
if (! defined $_) {
return unless $object->start_next_part;
return $object->read_next_node;
}
my (%header, %menu);
for my $label (qw(File Node Prev Next Up)) {
($header{$label}) = /$label:\s*([^,]*)/;
}
do {
$_ = <$fh>;
return %header if /^\037/ || ! defined $_
} until /^\* Menu:/ ;
while (<$fh>) {
my ($key, $ref);
last if /^\037/; # end of node
next unless /^\* \S/; # skip non-menu-items
if (/^\* ([^:]*)::/) { # menu item ends with ::
$key = $ref = $1;
} elsif (/^\* ([^:]*):\s*([^.]*)[.]/) {
($key, $ref) = ($1, $2);
} else {
warn "Couldn't parse menu item at line $.
of file $object->{NAME}";
next;
}
$menu{$key} = $ref;
}
return (%header, Menu => \%menu);
}
一个简单的示例程序，演示了该库的使用
use Info_File;
my $file = shift;
my $info = Info_File->open_info_file($file)
or die "Couldn't open $file: $!; aborting";
while (my %node = $info->read_next_node) {
print $node{Node},"\n"; # print the node name
}
综合起来
这次代码并没有缩小；它与之前一样大。有些部分变小了，但是转换到面向对象风格带来的开销使得代码再次变大了。
但面向对象风格为我们带来了几个重大优势。接口变得更好；库不再通过全局变量通信，也不再破坏 INFO。它还获得了同时处理两个或更多信息文件的能力，或者对同一个信息文件进行多次处理，这对于在大型项目中使用是必不可少的。灵活性也得到了提高：只需几行额外的代码就能提供搜索任何节点或通过名称回溯到节点的功能。
红旗
这次我们看到的红旗总结
计算机编程的基本规则是，如果你写了两遍相同的代码，你很可能做错了什么。至少，你可能会为自己以后在某人更改代码时设置维护问题。
编程语言中充满了旨在从最底层（例如，$a[3] += $b 而不是 $a[3] = $a[3] + $b）到最高层（例如，DLLs 和管道）防止代码重复的功能。在这之间是必要的功能，如子程序和模块。
每次你看到自己写了两遍相同的代码时，都要认真思考如何消除除一个实例之外的所有实例。
eof()
Perl 的 eof() 函数几乎总是不好的选择。它通常被初学者和编程时间过长的 Pascal 程序员过度使用。
Perl 通过返回一个未定义的值来返回一个明确的文件结束条件。Perl 的 I/O 操作符旨在使其方便地检查这一点。甚至 while(<FH>) 结构也会自动这样做。显式检查 eof() 几乎从未需要或可取。
return 0 和 return undef
这通常是一种尝试返回一个被调用者视为布尔假的值。但在列表上下文中，它会被测试为真，而不是假。除非函数 总是 在列表上下文中返回单个标量，否则通常使用普通的 return; 来产生假值是一个更好的选择。
一些程序员会写 wantarray() ? () : undef，它做的是同样的事情，但更冗长且更令人困惑。
简要忏悔
本文讨论的程序确实是由一个Perl新手编写的。我在1993年编写了它，当时我只用Perl编程几个月。我一定很满意，因为这是我第一次在公开论坛上发布的Perl程序。
标签
反馈
这篇文章有什么问题吗？请在GitHub上打开一个问题或拉取请求来帮助我们：GitHub
