重构遗留代码(2):魔术字符串和常量

旧代码,丑陋的代码,复杂的代码,意大利面条似的代码,鬼话废话……就是四个字:遗留代码。这是一个系列文章,将有助于你处理并解决它。

我们第一次见到遗留代码是在我们的上一篇教程。然后我们运行了代码来对其基本功能形成一个认识,并且创建了金牌大师测试套件来为未来的代码修改修建一张安全网。我们将继续处理这个代码,你可以在trivia/php_start这个目录下找到它。另一个目录trivia/php_start则包含了这次教程的完成代码。

做出第一次改变的时机已经来到,比起理解一段艰难的代码库来说,着手将魔术常量和字符串抽出到变量中是不是不失为一种更好的方式?这看似简单的任务将带给我们对于遗留代码的内在作用机制更广泛的有时是意想不到的洞见。我们需要弄清楚原代码开发者的意图,并且为我们之前不曾见过的代码片段找个合适的名称。

 

字符串

魔术字符串是没有被赋值给变量的,直接用在各种表达式中的字符串。这种类型的字符串对于原开发者的代码来说有特殊意义,但取代把它们赋值给一个适当命名的变量来说,原开发者认为该字符串的含义是显而易见的。

 

识别第一个需要修改的字符串

让我们开始在Game.php中寻找并识别字符串。如果你在用IDE(并且你应该用)或者有高亮显示代码功能的智能文本编辑器,定位字符串将会容易些。这里是在我的显示器上代码的样子。

每一个橘色的都是字符串。通过这种方式找到我们代码中的每个字符串是很容易的。所以,无论你的应用程序是什么,确保你的编辑器支持高亮并且可用。

我们代码中第一块橘色的部分立即显示在第三行。但是这个字符串仅仅包含一个换行符。这在我看来应该显而易见,所以我们继续。

当要决定什么该抽取,什么该保留不变时,很少有赞成的,但最后你的专业意见必须占上风。基于这点,你将必须决定对你所分析的每一块代码做什么。

那么让我们来看看32到42行,就是你看到的上面的片段。对于pop,science和sports questions,仅仅是一个简单的拼接。然而,为rock question组成字符串的动作被抽出到一个方法中了。在你看来,这一系列的字符串足够清晰以至于我们可以将它们保留在我们的for循环中吗?或者,你认为抽取所有的字符串到它们的方法中将证明那些方法的存在?如果这样的话,你将如何命名那些方法?

更新金牌大测试程序

不管结果如何,我们都需要修改代码。是时候让我们的金牌大师运行,编写能实际运行的测试程序,并将我们的代码和现有的内容进行比较了。

我们创建了另一个测试程序去与输出比较,确保testGenerateOutput()仅产生一次输出并且不再做别的。我们也把金牌大师的输出文件"gm.txt" 放到当前目录,因为"/tmp"目录当系统重启时可能会清空。对于我们实际的结果,我们仍将使用它。在大多数UNIX类系统中"/tmp"被安装到RAM使得它比文件系统快得多。如果你做的足够好,测试程序将正常通过。

为将来的修改,记得将我们的生成器程序标记为“跳过”是很重要的。如果注释它或甚至是全部删除它能让你感觉更舒坦的话,那么请这么做。重要的是,当我们修改代码时,金牌大师将不会变。它是一次生成的并且我们不想修改它,永远不,这样我们就能保证新生成的代码总能和原始的进行比较。如果你觉得对它做个备份更舒适的话,请继续这么做。

我将仅标记测试程序为跳过。这将使我们的测试结果变为"yellow",意味着所有的测试程序都通过但是一些是跳过或者标记为未完成的。

 

做出第一个修改

测试程序就位,我们可以开始改代码了。根据我的专业意见,所有的字符串都可以被保留在for循环里。我们将把createRockQuestion()方法的代码移到for循环中,并且删除这个方法。这种重构称为内联方法

“把方法代码放到它的调用者的方法体中并删除该方法” ~ Martin Fowler

有一套具体的步骤去做这种类型的重构,如同Marting Fowler在《重构:改善既有代码的设计》中定义的那样:

  • 检查该方法没有多态性。
  •  找到所有调用该方法的调用者。
  • 用方法代码替换每一个调用者的代码。
  • 编译并测试。
  • 删除方法定义。

我们没有子类继承 Game,所以第一步有效。

只有一个使用我们方法的调用者,在for循环内。

在构造器中,我们将方法createRockQuestion()的代码放到for循环中。我们仍保留旧代码。现在是时候运行我们的测试了。

我们的测试通过了。可以删除createRockQuestion()方法了。

最后我们应该再次运行测试程序。如果我们漏了对方法的调用,那么测试将会失败。

测试程序应该再次通过。恭喜!我们完成了第一个重构。

 

另一些需要考的字符串

add()roll()方法中字符串仅仅通过调用echoln()方法输出它们。askQuestions() 方法比较字符串类型。这看起来也能接受。另一方面,currentCategory() 基于数字返回字符串。这个方法中,有许多重复的字符串。仅仅在这个方法中,改变除了Rock以外的任何类别都需要在三处修改它的名字。

我认为我们能做得更好。我们可以使用一种称为引入局部变量的重构方法并清除重复。下面是方针:

  • 增加一个变量并赋上想要的值。
  • 找到所有使用这个值的地方。
  • 替换所有使用这个值的为该变量。

这好很多了。你可能已经注意到一些我们可以对代码在未来能做的改进,但我们才刚开始我们的作业。你可以立即修正你发现的一切这点是很诱人的,但请别这么做。许多时候,特别是在代码被很好地理解之前,这种修改会引向死胡同甚至更烂的代码。如果你觉得你有可能会忘掉你的想法,就在便利贴上或你的工程管理软件中创建一个任务记下来。现在我们需要接着处理我们的字符串相关问题了。

在接下来的文件中,所有的字符串都是输出相关的,放入echoln()方法。目前这个阶段,我们不会去碰它们。修改它们可能会影响我们的应用程序的打印和传递逻辑。它们是表示层和业务逻辑混合的一部分。我们将在将来的教程中分别处理不同的关注点。

 

常量

魔术常量和魔术字符串很像,但是值不同。它们的值可以是布尔值或者数字。我们将专注于在if表达式或者 return 表达式或其他的表达式中使用数字。如果这些数字没有明确的意义,我们需要将它们抽出到变量或者方法中。

这次我们将以GameRunner.php开始,我们首先关注提供一些随机数的roll()方法。之前的开发者没有在意给那些数字以意义。我们可以吗?如果我们分析代码:

它会返回1到6之间的数字。随机部分返回0到5之间的数字,我们总是加1。所以可以肯定在1到6之间。现在我们需要考虑我们的应用程序的内容了。我们开发了一个问答游戏。我们知道有某种类型的告示板标示我们的参与者必须移动。为了做到这点,我们需要掷骰子。骰子有六个面并且它可以产生1到6之间的数字。这似乎是个合理的推论。

这不是很美妙吗?我们又一次使用了引入局部变量的重构概念。我们命名新的变量$dice并且它代表产生1到6之间的随机数。这也使得我们接下来的表述听起来很自然:玩游戏,掷骰子。

你运行你的测试程序了吗?我没提到它,但我们需要尽可能频繁得运行它们。如果你还没有,那么此时将是个很好的时机跑起来。它们应该是通过的。

所以,这无非只是一个交换数字变量的例子。我们让一整个表达式代表数字并把它提取到一个变量中。这从技术上可以认为是魔术常量的例子,但并不是一个完全的例子。我们下面的随机表达式怎么样?

这个更棘手。0,9和7在这个表达式中是什么?也许我们可以给它们命名。乍一看,我对0和9毫无办法,那么我们试试7。如果我们随机方法返回的数字等于7,那么我们将进入if表达式产生错误答案的第一个分支。所以也许这里的7可以命名为$wrongAnswerId。

我们的测试程序仍能通过并且代码有点更具表达力了。现在我们试图给数字7命名,它将条件引入不同的上下文中。我们也可以考虑为0和9起一些合适的名字了。它们仅仅是rand()方法的参数,所以变量可能命名为min-什么和max-什么。

现在就很有表达力了。我们有了一个minAnswerID,一个maxAnswerID和另一个wrongAnswerId。谜题解决了。

但注意所有的代码都在do-while循环中。我们需要每次重设回答ID变量吗?我认为不需要。让我们试着将它们移出循环然后看看我们的测试程序能不能通过。

是的,测试程序这样也能通过。

是时候转向Game.php并且看看那儿的魔术常量了。如果你有代码高亮功能,你一定有些鲜亮的颜色突显常量。我的是蓝色的并且他们很容易找到。

在这段for循环中找到50这个魔术常量是非常容易的。如果我们看一下代码的作用,我们可以发现在for循环中,元素被放入几个数组中。所以我们有了一些列表,每个都有50个元素。每个列表代表一个问题集,变量实际上是在上面数组类的字段。

那么,50能代表什么?我猜你已经有了些想法。命名是编程中最难的任务之一,如果你有不止一个想法而你不确定该选择哪一个的时候,不要感到难为情。我的脑海中也有许多名字并且我正在评估选择最好的一个的可能性,甚至是在写这段的时候。我认为我们可以为50选择一个保守的名字。有时是$questionsInEachCategory或者$categorySize或者一些相似的。

这看起来很像样。我们可以保留它。测试程序当然也是通过的。

2是什么?我确信这时候答案对你已经显而易见了。很容易:

你这么认为吗?如果你有更好的想法,请在下面自由评论。你的测试程序呢?他们仍然能通过吗?

现在,在roll()方法中我们也有一些数字:2,0,11和12。

这非常清晰。我们将把表达式放进方法中,不过不在这篇教程中。我们仍然在理解和搜寻魔术常量和字符串的阶段。那么11和12怎么样?它们被埋在if表达式的第三层。很难理解它们代表什么。也许我们该看看它们周围的代码。

如果当前玩家的地方或位置大于11,那么它的位置将减少为现在的数字减去12。这听起来像是当玩家到达告示板或游戏区的末尾,并且它会被重新定位到初始位置。也许是位置0。或者,如果我们的游戏告示板是圆形的,那么走到最后一个位置的将使玩家置于相对的第一个位置。所以11可能是告示板的长度。

别忘了在方法中要替换两处11。这将迫使我们将变量赋值移到if表达式外面,就在第一个缩进级别。

那么如果11是告示板的长度,那么12是什么?我们从当前玩家位置减12,而不是11。并且为什么我们不直接将位置设为0来替代减法?因为这可能使测试程序失败。我们之前的猜想,当在if表达式中的代码运行后,玩家将在位置0终结是错误的。让我们假设玩家在位置10,骰子是4。14比11大,那么减法就会执行。然后玩家的终结位置是10+4-12=2。

这将促使我们走向对11和12的另一个可能的命名。我认为将12命名为$boardSize更合适。但这么做对于11来说留给我们的是什么?也许是$lastPositionOnTheBoard?有点长,但它至少告诉了我们这个魔术常量的真相。

我知道,我知道!有些代码是重复的。很明显,特别是剩下的代码隐藏了。但请记住我们在处理魔术常量。也会有时间处理重复代码的,但不是现在。

写在最后

我在代码里留了最后一个魔术常量。你能找到它吗?如果你查看最终代码,它已被替换掉,不过当然这是作弊了。祝你好运找到它并感谢你的阅读。

收藏 评论

关于作者:EluQ

南京土著,程序员。(新浪微博:@EluQ) 个人主页 · 我的文章 · 11

相关文章

可能感兴趣的话题



直接登录
跳到底部
返回顶部