程序维修店归来及红旗问题
红旗问题 - 消除合成代码 - 注意循环中的特殊情况 - 不要对数字应用字符串操作
反编程
几周前,我收到了Bruce的邮件,他是一位前学生,想取一个像12345678
这样的数字,它来自数据库,并将其格式化为12,345,678
以在报告中打印。我把他推荐给了perlfaq5
手册页中的解决方案,但那里的解决方案使用了一个相当奇怪的重复正则表达式,也许这就是他决定自己来做的原因。这是他给我看的代码
1 sub conversion
2 {
3 $number = shift;
4 $size = length($number);
5 $result = ($size / 3);
6 @commas = split (/\./, $result);
7 $remain = ($size - ($commas[0] * 3));
8 $pos = 0;
9 $next = 0;
10 $loop = ($size - $remain);
11 while ($next < $loop)
12 {
13 if ($remain > 0)
14 {
15 $section[$pos] = substr($number, 0, $remain);
16 $next = $remain++;
17 $remain = 0;
18 $pos++;
19 }
20 $section[$pos] = substr($number, $next, 3);
21 $next = ($next + 3);
22 $pos++;
23 }
24 $loop = 0;
25 @con = ();
26 foreach (@section)
27 {
28 $loop++;
29 $cell++;
30 @tens = split (/:/, $_);
31 $con[$cell] = $tens[0];
32 if ($loop == $pos)
33 {
34 last;
35 }
36 $cell++;
37 $con[$cell] = ",";
38 }
39 return @con;
40 }
Bruce将其描述为“可能相当原始和庞大。”我必须同意。40行代码已经接近一个可读函数的极限,而且没有理由让这样简单的东西变得如此庞大。Bruce在这里做了很多编程工作,产生了大量的代码;让我们看看我们是否能取消一些代码,并最终得到比开始时更少的代码。
5 $result = ($size / 3);
6 @commas = split (/\./, $result);
7 $remain = ($size - ($commas[0] * 3));
最前面可能是整个程序中最奇怪的一块代码。我知道它很奇怪,因为当我第一次看到它时,我立刻就明白了它的作用,但当我几周后再次查看程序时,我完全无法理解。Bruce知道原始数字中的数字将被分成三组,并在开头有一个剩余的数字组。他想知道第一组中可能有多少个数字,可能是零。为了做到这一点,他需要将数字除以三,并找到余数。
Bruce在这里做了一件非常巧妙的事情:这里的代码将$size
除以3,并假设结果将是一个十进制数。然后它通过使用小数点字符进行分割来获取整数部分!
Perl已经有了一个更简单的方式来获取余数:%
运算符
$remain = $size % 3; # This gets the remainder after division.
还值得记住的是,Perl有一个int()
函数,它丢弃数字的小数部分并返回整数部分。这本质上就是这里使用的split
所做的事情。
一个有用的经验法则是,将数字当作字符串处理是特别的,每次在数字上执行模式匹配时,都应该怀疑。几乎总是有更自然的方法可以得到相同的结果。例如
if ($num =~ /^-/) { it is less than zero }
是奇怪的并且晦涩难懂的;它应该是
if ($num < 0) { it is less than zero }
8 $pos = 0;
9 $next = 0;
10 $loop = ($size - $remain);
11 while ($next < $loop)
12 {
13 if ($remain > 0)
14 {
15 $section[$pos] = substr($number, 0, $remain);
16 $next = $remain++;
17 $remain = 0;
18 $pos++;
19 }
20 $section[$pos] = substr($number, $next, 3);
21 $next = ($next + 3);
22 $pos++;
23 }
现在这里有一个带有内部if
条件的while
循环。这个if
条件是$remain
为正。在if
块内部,$remain
被设置为0,并且在这个代码段的任何其他地方都没有改变。所以我们可以推断出,这个`if`块只会执行第一次循环遍历,因为之后,$remain
将是0。
这表明我们应在开始循环之前先执行if
部分,因为这样我们就无需每次都测试$remain
。然后结构会更简单,因为我们可以把if
块从while
块中移出,而且还会稍微短一些,因为我们不需要管理$remain
的代码。
$next = 0;
$pos = 0;
if ($remain > 0)
{
$section[$pos] = substr($number, 0, $remain);
$next = $remain + 1;
$pos++;
}
$loop = ($size - $remain);
while ($next < $loop)
{
$section[$pos] = substr($number, $next, 3);
$next = ($next + 3);
$pos++;
}
在while
循环中,我们看到另一个我在上一篇文章中指出的常见初学者错误的例子。每当有一个变量如$pos
,它只存在以跟踪数组末尾的位置时,你应该去掉它。例如,这里$pos
的唯一用途是在@section
末尾添加新元素。但push
函数已经做到了这一点,无需使用$pos
。每当有看起来像的代码
$array[$pos] = SOMETHING;
$pos++;
你应该看看是否可以用以下代码替换
push @array, SOMETHING;
97%的情况下,它是可以替换的。这里的结果是
$next = 0;
if ($remain > 0)
{
push @section, substr($number, 0, $remain);
$next = $remain + 1;
}
$loop = ($size - $remain);
while ($next < $loop)
{
push @section, substr($number, $next, 3);
$next = ($next + 3);
}
在代码的这个阶段,我得到了一个灵感。$pos
只是更普遍原则的一个特殊情况。在每一个程序中,都有两种代码。每一段代码要么与手头的问题有自然联系,要么是偶然的副作用,因为你恰好使用数字计算机来解决该问题。第二种代码只是脚手架。所有编程的目标都是减少这种偶然的合成代码,以便自然、本质的代码更明显。
$pos
是合成代码的一个完美例子。它与给输入添加逗号无关。它只存在,因为我们恰好使用数组来存储原始输入的块,而数组是按数字索引的。数组索引变量几乎总是合成的。
好的语言提供了许多减少合成代码的方法。这里有一个例子。假设你有两个变量a
和b
,你想要交换它们的值。在C语言中,你不得不声明一个第三个变量,然后这样做
c = b;
b = a;
a = c;
这里的多余变量完全是合成的。它与你想做的真正事情毫无关系,那就是交换a
和b
的值。在Perl中,你可以这样写
($a, $b) = ($b, $a);
并省略合成变量。
有趣的是,有时候一旦你给它起了一个名字,思考某件事就会变得容易得多。一旦我对合成代码有了这个灵感,我立刻开始到处看到它。我立刻注意到$next
和$loop
是合成的,我开始想是否可以去掉它们。不仅它们都是合成的,而且它们被用于相同的目的,即控制while
循环的退出。控制一个循环的两个变量是过度的;在大多数情况下,你只需要一个变量来控制一个循环。如果有两个,就像在这个例子中,几乎总是可以消除一个或合并它们。结果发现,$loop
完全没有用,我们可以用$size
代替。因为$size
很简单,它只是输入的长度,而使用length($number)
则更自然。
if ($remain > 0)
{
push @section, substr($number, 0, $remain);
}
$next = $remain;
while ($next < length($number))
{
push @section, substr($number, $next, 3);
$next += 3;
}
现在while循环的条件更容易理解,因为没有那个奇特且无意义的$loop
变量:“当字符串中的当前位置不是末尾时,获取另一个部分。”
我还将$next = $next + 3
改为$next += 3
,这更简洁。
现在我们有两个变量$next
和$remain
,它们只在一次重叠,而且在那次重叠(赋值)中它们代表相同的意义。所以让我们让一个变量做两个变量的工作
if ($remain > 0)
{
push @section, substr($number, 0, $remain);
}
while ($remain < length($number))
{
push @section, substr($number, $remain, 3);
$remain += 3;
}
代码将不会比这更简单了。我们将十二行代码简化为五行。
组装结果
24 $loop = 0;
25 @con = ();
26 foreach (@section)
27 {
28 $loop++;
29 $cell++;
30 @tens = split (/:/, $_);
31 $con[$cell] = $tens[0];
32 if ($loop == $pos)
33 {
34 last;
35 }
36 $cell++;
37 $con[$cell] = ",";
38 }
39 return @con;
40 }
在这里,我们想要从包含章节的列表中构建结果列表@con
。我不明白@tens
数组是做什么用的,或者代码为什么寻找:
字符,因为在数字中通常不会出现这些字符。原始程序将1234:5678
转换为123,4,678
,我不敢相信这是预期的结果。我向Bruce询问他在这里做什么,但我没有足够的上下文来理解他的回答——我觉得这是一个未完全实现的功能。所以我将其移除,并留下了一条注释。
$cell
是另一个变量,它唯一的作用是跟踪数组的长度,因此我们可以通过使用push
来消除它,就像我们之前做的那样。
foreach (@section)
{
$loop++;
push @con, $_;
# Warning: no longer handles ':' characters
if ($loop == $pos)
{
last;
}
push @con, ',' ;
}
return @con;
现在,$loop
变量的唯一用途是在向最后一个元素添加逗号之前退出循环。让我们简单地去掉它。然后当我们退出循环时,数组末尾会多出一个逗号,但清理多余的逗号比跟踪循环要容易。
foreach (@section)
{
push @con, $_, ',';
}
pop @con;
return @con;
再次,我认为这个循环不会变小很多。
现在我们有的看起来是这样的
# Warning: No longer handles ':' characters
sub conversion
{
my ($number) = @_;
my $remain = length($number) % 3;
if ($remain > 0)
{
@section = (substr($number, 0, $remain));
}
while ($remain < length $number)
{
push @section, substr($number, $remain, 3);
$remain += 3;
}
foreach (@section)
{
push @con, $_, ',';
}
pop @con;
return @con;
}
这是一个很大的改进,但还有更多的改进空间。@section
是一个合成变量;它只在那里,这样我们就可以遍历它,然后我们再将其丢弃。最好直接构建@con
,而不必先构建@section
。现在代码如此简单,更容易看到如何做到这一点。在原始程序中,一个循环将输入分成块,另一个循环插入逗号。我们可以用单个循环替换这两个循环,这样就可以完成这两个任务,因此这
while ($remain < $size)
{
push @section, substr($number, $remain, 3);
$remain += 3;
}
foreach (@section)
{
push @con, $_, ',';
}
变成了这样
while ($remain < $size)
{
push @con, substr($number, $remain, 3), ',';
$remain += 3;
}
消除第二个循环意味着必须在该特殊情况if
块被调用时插入它自己的逗号。
if ($remain > 0)
{
push @con, substr($number, 0, $remain), ',';
}
消除第二个循环还有令人愉快的效果,那就是使函数更快,因为它不再需要两次遍历输入。函数的最终版本看起来像这样
# Warning: No longer handles ':' characters
sub conversion
{
my ($number) = @_;
my $size = length($number);
my $remain = $size % 3;
my @con = ();
if ($remain > 0)
{
push @con, substr($number, 0, $remain), ',';
}
while ($remain < $size)
{
push @con, substr($number, $remain, 3), ',';
$remain += 3;
}
pop @con;
return @con;
}
这是一个很大的胜利。一个30行的函数变成了一个12行的函数。以前,它有九个标量变量和四个数组;现在它有三个标量和一个数组。如果我们想,我们可以通过消除有些是合成的$size
,并在函数的其他部分使用length($number)
来进一步减少它。收益似乎较小,所以我选择不这样做。
在好的代码中,程序的结构与数据的结构相协调。在这里,代码的结构直接对应于我们试图产生的结果的结构。我们希望将输入,如12345678
,转换为输出,如12 , 345 , 678
。前面有一个单独的if
块来处理初始数字组的特殊情况,这个组可能与其他组不同,然后有一个单独的while
循环来处理其他组。
关于这段代码真正有趣的是,我几乎完全没有使用Perl的任何特殊功能。清理完全来自于重新组织现有的代码和删除不必要的项目。当然,Perl的功能,如push
,使得消除在其他语言中可能需要的合成变量和其他代码变得容易。
关于这个问题的更“Perl式”(并且不幸地是混淆的)解决方案,请参阅FAQ。
红旗
红旗是一个警告标志,表明有问题。当你看到红旗时,你应该立即考虑你是否有机会使代码更简洁。我喜欢这个程序,因为它提出了许多红旗。
消除合成代码
您的程序中的一些部分直接与您试图解决的问题相关。这是自然代码。但程序中的某些部分只与其他程序部分相关;这是合成代码。一个例子是循环控制变量。从其名称可以判断它是合成的。它不是为了解决您的问题而存在的;它存在是为了控制循环,而循环的存在是为了帮助解决问题。您可能关心循环,但控制变量是一个不便之处,仅仅是为了记录。
注意循环中的特殊情况
如果您有一个具有特殊测试的循环,用于在第一次或最后一次迭代中执行某些操作,您可能可以消除它。第一次迭代代码通常可以提升到循环外的单独初始化部分。最后一次迭代代码通常可以提升并执行在循环完成后。如果循环运行了太多的代码,取消额外的操作通常比试图提前退出循环要简单。
不要对数字应用字符串操作
将数字视为数字串是一种奇怪的做法,因为数字本身与数字的值没有太多关系。这样做会创建一个数值量的字符串版本,这通常意味着您走错了路,因为Perl数字在内部以数值形式存储,应该支持您应该想要的全部数值操作。
如果您对一个数字使用了正则表达式,或 split
、substr
、index
、length
或任何其他字符串函数,这是一个危险信号。停下来考虑一下,是否可能有一种更自然和更健壮的方法,仅使用数值操作来完成相同的事情。例如,这很奇怪
if (length($number) > 3) { large number }
更自然的方式是写成这样
if ($number >= 1000) { large number }
这里有一个例外,当您确实将数字作为字符串处理时,例如当您将其写入固定宽度字段时。这两种情况在本文的程序中都有示例。在原文中,使用 split
来计算模运算符是不自然的。在最终版本中,我们确实对 $number
应用了 length()
和 substr()
,但这是因为我们真的想将数字作为数字字符串处理,将其分成三位一组并插入逗号。
尽管如此,危险信号仍然存在,因此我们应该看看如果我们遵循它,会发生什么,并尝试用真正的数值操作来替换 length()
和 substr()
。结果是
# Warning: No longer handles ':' characters.
sub convert {
my ($number) = shift;
my @result;
while ($number) {
push @result, ($number % 1000) , ',';
$number = int($number/1000);
}
pop @result; # Remove trailing comma
return reverse @result;
}
再次注意,从 @result
的末尾移除多余的逗号比特殊处理循环的第一次迭代以避免最初添加逗号要容易得多。现在代码有八行,一个标量和一个数组。我认为这可以算是一个胜利!这里要学到的教训是:当您不确定时,试着两种方式都写一写!
标签
反馈
这篇文章有什么问题?请通过在 GitHub 上打开一个问题或拉取请求来帮助我们。