重构遗留代码(6):进攻复杂的方法

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

在我们之前的五篇教程中我们花了大量的时间在理解遗漏系统上,在为任何我们能发现可测试的代码片段写测试程序上。我们到达了这样一个阶段,我们有相当多的可测试方法,但我们仍然避开复杂的难于理解的逻辑。是时候做一些认真的编码了。

理解roll()方法

我们第一个候选者是roll()方法。因为它不返回值,似乎很难弄清它做了什么和怎么测试它。当我不确定如何开始测试一个代码片段的时候,我会试着逐行读它并且一步步来理解它。有时这样是可以的,但有时代码就只是太复杂了。

当在读和学得过程中,我也会做一些我知道的IDE可以安全完成的重构。这些重构大部分是对我认为已理解的变量和方法的重命名,我想让它们对我和今后的阅读者更加显而易见。进一步考虑,我们的金牌大师仍可做偶尔的测试。

查看方法的签名:roll($roll),我在想$roll参数代表了什么?它是对象吗?它是动作吗?它是数字吗?我的IDE在这儿是有帮助的。通过只将鼠标放到$roll参数上,所有它的用法将以青色略微突出显示。

我们可以看到高亮的$roll变量在63,67和71行。而这些只是适应于屏幕出现的。尽管可能之后还有其它的使用,这三个是不错的候选者来帮助我们弄清$roll变量的角色。

在63行,该变量用来打印文本到屏幕。echoln(“They have rolled a” . $roll);。这行很容易理解,它也确实帮到了我们。它告诉我们某些玩家掷出了”$roll”。那么你怎么掷?你掷一个数字。也许我们可以重命名$roll为$number。这将使我们的方法签名看起来很自然:roll($number)。

但67行是关于什么的?如果我们把$roll重命名为$number,那么条件表达式在该方法的上下文中还有意义吗?

我不喜欢那样,如果我只看这段代码的话,我不知道$number代表了什么。方法的定义在5行之上。我可能已经忘了它,或者也许我根本没读过它。我们也在代码的第三个缩进层次,我们最初的语境已经改变如此之多了。也许需要一个更具描述力名字。$rolledNumber怎么样?那将解释它作为数字的事实,并且也将保持它在源代码中的名字。我们知道它是玩家掷出的一个数字。我们知道它可以在0到6之间。这重要吗?也许,毕竟,我们在试着理解一个遗留系统。

既然我们已经解决了参数的命名问题,我们了解到下面的两行仅仅是输出文本,我们可以继续分析我们的第一个if表达式了。在它之前还有一个变量赋值,但我们还不会关心那个。

if表达式的第一部分相当大。更确切得说有20行之长,从66到86行。这要吸收相当多的信息。也许”else”部分短些。我们可以向下滚动看看它是不是很容易理解。这部分只有10-12行。并且它们的一般都是输出,或者为空行,所以我们可以发现这里没有太多的逻辑。也许它值得去分析一下。

if中的第一行似乎是在告示板上将当前玩家重置到一个新的位置。它通过掷骰子让玩家前进。这是典型的棋牌游戏,并且听上去很合乎逻辑。

然后我们又有了一个条件,但只是一个简单的if,检查玩家是否开始了新的一轮。如果是的,那么我们将当前玩家放到相符的位置。你回想得起当我们简化if表达式时,抽取了一个方法并且命名为playerShouldStartANewLap()?可能是两个多月之前了。对现在理解这段逻辑来说多么有帮助的一小步啊!

最后,在最后一行代码上,某些信息被显示出来,问题被提出来。

哇哦。我刚意识到我们可以以一句话来解释这里发生了什么:“玩家根据掷骰子的数字移动到下一个位置,信息显示给用户,并且我们问一个问题。”。当我可以这么做的时候,我急于快速为每个我刚刚确定的部分创建一个方法。三个简单的方法已经在我的脑海中漫游着。虽然我完全可以靠我的IDE的能力抽取方法,但对这部分代码做一些测试会让我感到更舒服些。我们可以以某种方式准备一个合适的玩家的game对象,在合适的条件下,以便if表达式的第二部分可以被触发?

 

测试roll()的第二部分

测试遗留代码的难处之一,是将SUT(待测系统)置为适当的状态,以便我们可以验证感兴趣的状态。我们已经知道初始化一个Game类是容易的。不需要构造函数参数。那么,如果我们查看类变量的列表。它们简单得用var关键字定义。在PHP中这意味着它们被认为是公共变量。我们已经在前面的测试中用到了currentPlayer,所以我们可以确定我们能从对象外部访问变量。

此刻,我要开始写测试程序了。不是完整的测试,仅仅足够我来弄清楚SUT是如何执行的。

测试的名字还不明确。我们知道在if表达式的else部分里做了三件事,但我们只以两到三个词来定义这个并不真正清楚。所以,暂时,我们可以用某物来描述我们的想要执行的目标代码片段。如果需要的话,稍后我们将抽出时间重构名称。

然后我们根据代码需要设置变量。基本上,我复制黏贴变量并且在每个$this->后面加上game。然后我用一个数字调用roll()。这个数字目前是不相关的,我只是随意选了数字1。

虽然这个代码没有断言,但我们可以通过观察输出弄清楚哪一部分代码在运行。

我们看到“John”是我们的当前玩家,正如我们在测试程序的上面几行所定义的,然后我们可以识别只存在if表达式的else部分使用的关键字符串:“new location is”。

那么,输出帮助我们确认我们是谁,我们应该在哪儿。下一步是弄清楚我们应该在测试程序中验证什么。

我们可以确认当不应该开始新的一轮的玩家的下一个位置。

好了。我们在那儿写了不少新的代码。首先,我们定义了当前位置和骰子数变量。然后我们在告示板上用上面具体的位置设置了当前玩家的位置。在调用roll方法之后,我们可以验证不需要开始新的一轮的玩家的新位置,它是两个数字的和。

随着我们测试的通过,是时候做一点重构了。我们还不能在产品代码上做太多,但我们的测试程序需要一点爱(需要点改变)。

这时没什么花哨的,只是一些隐藏了泛滥于我们的类中丑陋的参数调用的抽出方法。这很容易理解,如果我们需要改变从Game类设置或读回必须信息的方式,我们不需要修改测试方法。我们只要修改私有方法。

下个问题是:“产品代码上我们还能测其他什么?”当玩家需要开始新的一轮时我们不想要进入if。这将使另一个测试的话题了。两个echoln()表达式输出到标准输出设备。关于那个我们可以测试一点。我们可以捕获输出并且对其进行测试,但它是显示。我们已经可以感觉到这里有嵌入到业务逻辑中的表示层,但我们不能清楚地知道如何安全地抽出它。所以,暂时地,就把它留在那儿不测试。接着有个对askQuestion()的调用。我们必须验证这个方法做了什么?和我们能否以某种方式测试它。

askQuestion()验证当前类别然后对用户输出包含问题的字符串。当前类别是有currentCategory()方法定义的,它简单得测试了当前位置,以及它是否响应了一个具体的数字,一个类别是否被选。在我们的测试程序中,我们使用数字3来响应“Rock”类别。askQuestion()只在屏幕上输出。另一个显示物我们还不想测试。但currentCategory()返回一个字符串,对askQuestion()来说字符串是必不可少的。也许我们可以调用currentCategory()并确保返回适当的类别?

我们增加的最后一行就是做的这个。看起来我们成功得测试了目标代码片段的所有功能。现在,我们也许要开始重构产品代码了。

但等一下!我们需要开始新的一轮的情况怎么办?在接触产品代码前,我们不该也测试那个吗?我想目前继续测试是个好主意,而当我们有把握没有破坏任何东西时再重构产品代码。

我们复制黏贴前面的测试程序,相应得更名并且制定不同的位置和一个骰子数。我们知道告示板的大小是12个位置。我们从位置11掷出2以便在位置1终止。告示板计数从位置0开始。

但我们第二个断言失败了。类别是“Science”。这次测试突出了我们方法的几个问题:1)我们需要重命名第一个测试程序,并且2)我们需要在一个不同的测试程序中测试类别。那么又是重构时间了。

我们重命名了第一个测试程序以反映它所验证的确切的事。在两个测试中我们移除了类别的验证。我们知道我们有两个不同的类别和两个位置。根据我们所知道的和currentCategory()方法的结构,我们可以推断将有一些包含各种类别的地方。首先我们在数组中定义位置,接着我们期待两个类别有两个不同的值。

此刻,我们的目标不是测试currentCategory()方法。我们可以停止当前进程给所有位置和类别的组合写测试程序。但我还不想那么做。现在,我们必须仍然关注roll方法和我们的小段代码。我们还能移除两个测试程序之间的重复并且抽取验证到一个私有方法。这将对我们将来写currentCategory()测试有帮助。

 

重构roll()的第二部分

既然我们所有的测试程序已经出色地写好并通过了,修改源代码就是下一个逻辑步骤。包含玩家移动逻辑的if表达式必须首当其冲。

看起来我们的方法至少需要两个参数。告示板大小和骰子点数。余下的使用信息来自类变量,所以它们不需要作为参数传递。然而,告示板大小看起来也像是属于方法的一个值,而不是属于game类的。稍后,我们将看看是否可以将其移到方法内。

接下来,代码行显示的是用户必须进到他们自己的小的具体方法中。我们本可以将所有的显示物放入一个方法中,但是会很难命名它。最好是有更好的命名和更小的方法。

这么做,我么就完成了这部分代码。但剩下的roll()方法部分怎么办?

 

两个开发者,两个不同的线程

直到现在,这篇教程关注的重点是小的代码段。我们的缩放级别很接近这段代码。我们做了很多事去思考8到10行以内的代码。我们专注于变量名或者移动一到两行代码从一处到另一处这样的小事。

我们在Syneto做许多结对编程。基本上每次至少有一个中等难度的任务要去完成时,我们就结对。重构是相当难的任务,所以大部分时间都有两双眼睛盯着代码。这样允许我们通过在两个不同的缩放级别上观察代码来理解和思考我们在做的事。当一个开发者做微小的修改并且专注于字符或者行级别的细节时,另一个人可以远处观察代码。

虽然这些网页的格式并不允许大量横向空间,但在现实中,任何开发者都应该至少有个25寸的显示器,这样是允许一个更高的缩放级别的代码视图的合适的解决方案。这里是从我的27寸显示器上看到的roll()方法。

在这个级别上,一个人可以观察形式,缩进和替换。

这个人可以思考复杂性,代码设计和可以被抽出的可能方法。这个人可以评估逻辑的复杂性,另一个人同时做了微笑的修改来处理代码的复杂性。更高的试图也可以相当有效得突出显示重复。

这不是很酷吗?在这张图之前你看过如此巨大的重复吗?你可以同时专注于微小细节和高级别视图吗?也许你是这样。一些人有天赋去理解代码,但我们大多数人不能有效专注于多个层次。

它最好的部分是什么?你也可以单独这么做!就是说,如果你没有机会与另一个开发者结对编程,你仍然可以放大和缩小。但你必须按顺序去做以便有效率。像这样的事情就是我们在开始做的。我们在一个更高的层次上观察方法,我们寻找可以击破的小段代码并且放大它。关注点的改变有效地转换了我们的心态和思考的方式。我们保持着放大的状态,直到我们对它完成重构之前并不去想剩下的代码。现在我们可以再次缩小它,再次转换我们的心态并且继续了。

 

重构roll()的第一部分

有些人在不到一分钟的时间里很容易得变换了心态和观点。另一些人需要更多的时间来“忘记”细节并且抽身出来,或者相反,放弃思考形式并开始一次阅读一整章代码。如果你需要10到15分钟,并不如你想的那么罕见。试着以允许你把下一个十分钟用来打破缩放界限这样的方式来组织你的工作。

并且在此,转换心态,我们需要放大if表达式的第一部分,当用户在禁区的情况。

这段代码以另一个if()表达式开始,检查骰子数是否是奇数。如果它是,它就做些复杂的事情。如果它是偶数,它就做些相当简单的事。只是将玩家保持在禁区。

那将很容易测试并且让我们专注于定义设置我们SUT的方式上。

是的,很容易。漂亮的四行测试方法。并且setAPlayerThatIsInThePenaltyBox()方法和它的对手很相似。只有禁区状态不同。

现在当骰子数是奇数时,我们可以开始为第一部分额if构建一个测试,或者几个测试程序。

这是有希望的开始。第一行:测试了。剩下来的将和第一层的if的else部分的测试程序相似。

 

结对应该直到最后

在Syneto我和我的同事面临的一个困境是,当我们结对编程或者重构像这样的事情时,当任务变得清晰并且即将完成时,开发者中的一员就想要离开了。

如果你是那个专注于写测试程序和移动小段代码的人,你的同伴可能会想他的角色已经完成了。他找到高层次的问题,将它们传达给你,现在就轮到你来修正这些代码了。当他试着离开时,阻止他。告诉他所有的任务开始于结对也该结束于结对。告诉他让他帮你思考测试程序,命名,结构,以便你能够专注于过程和步骤等需要用到的重构技术,就是那些我们在之前教程中学到的步骤。

举例来说,他可以考虑测试命名,而你忙于复制/黏贴和修改前一个测试,迫使进入这个if部分。

另一方面,如果你的同伴决定在发现所有的重复和厉害的高层次问题之后,他应该有权去写测试程序和重构代码,你可能会觉得你没什么其他事可做了。但你错了。你可以留下并在命名,低层次的重复和其他小事上帮助他。你也可以在他磕磕绊绊或者忽略了一些小的步骤或者当他因为一个愚蠢的小错字导致测试失败而受阻时帮助他。

这就是测试程序如何被很好命名的,就像testPlayerGettingOutOfPenaltyNextPositionWithNewLap()和变量将表达对于当前测试程序它们所代表的含义,而不是你拷贝的之前的测试程序锁做的:$numberRequiredToGetOutOfPenaltyBox。

这不比之前更好吗?所有的单元测试都通过了。但我感觉我们已经移动了许多用于演示的代码。也许我们应该运行金牌大师测试程序了?

 

回退一步

它失败了。自我们运行金牌大师测试程序已经过去很长时间了,但我们所有的修改都在roll()方法上。所以会发生的最糟糕的事是我们恢复我们的修改。

让我们开始往回一小步。我怀疑在我们看到的输出中的重复,有一个小的差异。也许是我们没观察到的一个字母或者一个空格。我们可以恢复roll()的第一部分的输出看看它是否起效。

仍然失败了。正如我们的第一个疑虑是错误的,我们可能想要回退一大步。金牌大师在我们开始这个重构之前是通过的吗?也许下次我们应该首先运行它。现在我们需要把我们的修改放在一个安全地地方并且恢复所有的代码以验证我们的假设。

对roll()最初状态的恢复让金牌大师通过了。很高兴知道这点。那么我们破坏它了。但什么时候?什么地方?

既然我们的代码恢复到原始状态,我们可以观察输出,将其放到文本文件并且将它和重构的那个进行比较。

正如我们可以立即观察到的,在重构版本中我们遗漏了一些行。字符串告诉我们玩家出禁区的部分遗漏了。嗯…

让我们再看看我们开始的代码。啊!!!在那儿!

echoln()停留在移动逻辑的顶部了。一个直率的,简单的错误。我们没有注意它而只是将所有代码块替换为方法调用。

这使得所有的测试程序通过了。谢天谢地我有个同伴帮助我发现这个问题。虽然我的同伴是一只泰迪熊,当我独自写这些文章的时候,许多时候它只是帮助告诉其他人你的问题。它将是你的头脑重演所有的思绪并且重造过程。还不止这样,这使你意识到愚蠢的错误并且发现你可能错过的事情。

 

添加最后的修改

在我们总结这篇教程之前,我们应该确保roll()方法已经是最佳状态。首先,所有的echoln()调用都放进私有方法中。

以上是在正确的方向上迈出的一步,但我的同伴说我们做得更好。

我们可以将连续的显示功能分组到其他的显示功能中。

这不是更好吗?我们的方法可以只通过一个显示调用跟踪每条路径。

你还记得$boardSize吗?我们现在可以把它放到movePlayer()里面了吗?是的,我们可以。那么,让我们做吧。

我们的代码变得相当短小了。但仍然,方法还有18行之长。那是很多的。你还记得Robert C. Martin的教程或者“魔术数字7加减2”吗?我们的程序如果只包含四行代码那将更好。

往这个方向的第一步,是对于每一个可能的路径,将其减少为对单一的方法调用。

我们现在减到12行代码了。但我们可以做得更好。最里面的if可以到它自己的方法中。

 

我们完成了!

这么做我们在方法中将代码降低了7行。方法中只有5行,只有4行真正在做一些逻辑。现在这是一个合理的有样子的方法了,在这点上,我觉得停留在这儿很好。并且,这不仅仅是个例子。这是“抽取直到你放弃”,并且是在Syneto的项目中我们大部分方法的样子。这是个现实生活中的例子,并且你应该每天这样结束你的代码。在这我们也结束这篇教程。

请继续关注接下来的教程,我们将谈谈关于层并且我们将开始分割关注点。

收藏 评论

关于作者:EluQ

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

相关文章

可能感兴趣的话题



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