舊程式碼,醜陋的程式碼,複雜的程式碼,義大利麵條似的程式碼,鬼話廢話……就是四個字:遺留程式碼。這是一個系列文章,將有助於你處理並解決它。
在我們之前的五篇教程中我們花了大量的時間在理解遺漏系統上,在為任何我們能發現可測試的程式碼片段寫測試程式上。我們到達了這樣一個階段,我們有相當多的可測試方法,但我們仍然避開復雜的難於理解的邏輯。是時候做一些認真的編碼了。
理解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,那麼條件表示式在該方法的上下文中還有意義嗎?
1 |
if ($this->isOdd($number)) { ... } |
我不喜歡那樣,如果我只看這段程式碼的話,我不知道$number代表了什麼。方法的定義在5行之上。我可能已經忘了它,或者也許我根本沒讀過它。我們也在程式碼的第三個縮排層次,我們最初的語境已經改變如此之多了。也許需要一個更具描述力名字。$rolledNumber怎麼樣?那將解釋它作為數字的事實,並且也將保持它在原始碼中的名字。我們知道它是玩家擲出的一個數字。我們知道它可以在0到6之間。這重要嗎?也許,畢竟,我們在試著理解一個遺留系統。
既然我們已經解決了引數的命名問題,我們瞭解到下面的兩行僅僅是輸出文字,我們可以繼續分析我們的第一個if表示式了。在它之前還有一個變數賦值,但我們還不會關心那個。
if表示式的第一部分相當大。更確切得說有20行之長,從66到86行。這要吸收相當多的資訊。也許”else”部分短些。我們可以向下滾動看看它是不是很容易理解。這部分只有10-12行。並且它們的一般都是輸出,或者為空行,所以我們可以發現這裡沒有太多的邏輯。也許它值得去分析一下。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 |
} else { $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $rolledNumber; if ($this->playerShouldStartANewLap()) { $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize; } echoln($this->players[$this->currentPlayer] . "'s new location is " . $this->places[$this->currentPlayer]); echoln("The category is " . $this->currentCategory()); $this->askQuestion(); } |
if中的第一行似乎是在告示板上將當前玩家重置到一個新的位置。它通過擲骰子讓玩家前進。這是典型的棋牌遊戲,並且聽上去很合乎邏輯。
然後我們又有了一個條件,但只是一個簡單的if,檢查玩家是否開始了新的一輪。如果是的,那麼我們將當前玩家放到相符的位置。你回想得起當我們簡化if表示式時,抽取了一個方法並且命名為playerShouldStartANewLap()?可能是兩個多月之前了。對現在理解這段邏輯來說多麼有幫助的一小步啊!
最後,在最後一行程式碼上,某些資訊被顯示出來,問題被提出來。
哇哦。我剛意識到我們可以以一句話來解釋這裡發生了什麼:“玩家根據擲骰子的數字移動到下一個位置,資訊顯示給使用者,並且我們問一個問題。”。當我可以這麼做的時候,我急於快速為每個我剛剛確定的部分建立一個方法。三個簡單的方法已經在我的腦海中漫遊著。雖然我完全可以靠我的IDE的能力抽取方法,但對這部分程式碼做一些測試會讓我感到更舒服些。我們可以以某種方式準備一個合適的玩家的game物件,在合適的條件下,以便if表示式的第二部分可以被觸發?
測試roll()的第二部分
測試遺留程式碼的難處之一,是將SUT(待測系統)置為適當的狀態,以便我們可以驗證感興趣的狀態。我們已經知道初始化一個Game類是容易的。不需要建構函式引數。那麼,如果我們檢視類變數的列表。它們簡單得用var關鍵字定義。在PHP中這意味著它們被認為是公共變數。我們已經在前面的測試中用到了currentPlayer,所以我們可以確定我們能從物件外部訪問變數。
此刻,我要開始寫測試程式了。不是完整的測試,僅僅足夠我來弄清楚SUT是如何執行的。
1 2 3 4 5 6 7 8 9 10 11 |
function testSecondPartOfRollLogic() { $this->game->currentPlayer = 0; $this->game->players[$this->game->currentPlayer] = 'John'; $this->game->inPenaltyBox[$this->game->currentPlayer] = false; $this->game->roll(1); } |
測試的名字還不明確。我們知道在if表示式的else部分裡做了三件事,但我們只以兩到三個詞來定義這個並不真正清楚。所以,暫時,我們可以用某物來描述我們的想要執行的目的碼片段。如果需要的話,稍後我們將抽出時間重構名稱。
然後我們根據程式碼需要設定變數。基本上,我複製黏貼變數並且在每個$this->後面加上game。然後我用一個數字呼叫roll()。這個數字目前是不相關的,我只是隨意選了數字1。
雖然這個程式碼沒有斷言,但我們可以通過觀察輸出弄清楚哪一部分程式碼在執行。
1 2 3 4 5 6 7 8 9 |
John is the current player They have rolled a 1 John's new location is 1 The category is Science Science Question 0 |
我們看到“John”是我們的當前玩家,正如我們在測試程式的上面幾行所定義的,然後我們可以識別只存在if表示式的else部分使用的關鍵字串:“new location is”。
那麼,輸出幫助我們確認我們是誰,我們應該在哪兒。下一步是弄清楚我們應該在測試程式中驗證什麼。
我們可以確認當不應該開始新的一輪的玩家的下一個位置。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
function testSecondPartOfRollLogic() { $currentPlace = 2; $rolledNumber = 1; $this->game->currentPlayer = 0; $this->game->players[$this->game->currentPlayer] = 'John'; $this->game->inPenaltyBox[$this->game->currentPlayer] = false; $this->game->places[$this->game->currentPlayer] = $currentPlace; $this->game->roll($rolledNumber); $this->assertEquals('3', $this->game->places[$this->game->currentPlayer], 'Player was expected at position 3'); } |
好了。我們在那兒寫了不少新的程式碼。首先,我們定義了當前位置和骰子數變數。然後我們在告示板上用上面具體的位置設定了當前玩家的位置。在呼叫roll方法之後,我們可以驗證不需要開始新的一輪的玩家的新位置,它是兩個數字的和。
隨著我們測試的通過,是時候做一點重構了。我們還不能在產品程式碼上做太多,但我們的測試程式需要一點愛(需要點改變)。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
function testSecondPartOfRollLogic() { $currentPlace = 2; $rolledNumber = 1; $this->setAPlayerThatIsNotInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $this->game->roll($rolledNumber); $this->assertEquals('3', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); } |
這時沒什麼花哨的,只是一些隱藏了氾濫於我們的類中醜陋的引數呼叫的抽出方法。這很容易理解,如果我們需要改變從Game類設定或讀回必須資訊的方式,我們不需要修改測試方法。我們只要修改私有方法。
下個問題是:“產品程式碼上我們還能測其他什麼?”當玩家需要開始新的一輪時我們不想要進入if。這將使另一個測試的話題了。兩個echoln()表示式輸出到標準輸出裝置。關於那個我們可以測試一點。我們可以捕獲輸出並且對其進行測試,但它是顯示。我們已經可以感覺到這裡有嵌入到業務邏輯中的表示層,但我們不能清楚地知道如何安全地抽出它。所以,暫時地,就把它留在那兒不測試。接著有個對askQuestion()的呼叫。我們必須驗證這個方法做了什麼?和我們能否以某種方式測試它。
askQuestion()驗證當前類別然後對使用者輸出包含問題的字串。當前類別是有currentCategory()方法定義的,它簡單得測試了當前位置,以及它是否響應了一個具體的數字,一個類別是否被選。在我們的測試程式中,我們使用數字3來響應“Rock”類別。askQuestion()只在螢幕上輸出。另一個顯示物我們還不想測試。但currentCategory()返回一個字串,對askQuestion()來說字串是必不可少的。也許我們可以呼叫currentCategory()並確保返回適當的類別?
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
function testSecondPartOfRollLogic() { $currentPlace = 2; $rolledNumber = 1; $this->setAPlayerThatIsNotInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $this->game->roll($rolledNumber); $this->assertEquals('3', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); $this->assertEquals('Rock', $this->game->currentCategory()); } |
我們增加的最後一行就是做的這個。看起來我們成功得測試了目的碼片段的所有功能。現在,我們也許要開始重構產品程式碼了。
但等一下!我們需要開始新的一輪的情況怎麼辦?在接觸產品程式碼前,我們不該也測試那個嗎?我想目前繼續測試是個好主意,而當我們有把握沒有破壞任何東西時再重構產品程式碼。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
function testAPlayerWillStartANewLapWhenNeeded() { $currentPlace = 11; $rolledNumber = 2; $this->setAPlayerThatIsNotInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $this->game->roll($rolledNumber); $this->assertEquals('1', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); $this->assertEquals('Rock', $this->game->currentCategory()); } |
我們複製黏貼前面的測試程式,相應得更名並且制定不同的位置和一個骰子數。我們知道告示板的大小是12個位置。我們從位置11擲出2以便在位置1終止。告示板計數從位置0開始。
但我們第二個斷言失敗了。類別是“Science”。這次測試突出了我們方法的幾個問題:1)我們需要重新命名第一個測試程式,並且2)我們需要在一個不同的測試程式中測試類別。那麼又是重構時間了。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 |
function testAPlayersNextPositionIsCorrectlyDeterminedWhenNoNewLapIsInvolved() { // ... // $this->assertEquals('3', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); } function testAPlayerWillStartANewLapWhenNeeded() { // ... // $this->assertEquals('1', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); } function testRockCategoryCanBeDetermined() { $currentPlaces = [3]; foreach($currentPlaces as $currentPlace) { $this->setAPlayerThatIsNotInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $foundCategory = $this->game->currentCategory(); $this->assertEquals('Rock', $foundCategory, 'Expected rock category for position ' . $currentPlace . ' but got ' . $foundCategory); } } function testScienceCategoryCanBeDetermined() { $currentPlaces = [1]; foreach($currentPlaces as $currentPlace) { $this->setAPlayerThatIsNotInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $foundCategory = $this->game->currentCategory(); $this->assertEquals('Science', $foundCategory, 'Expected rock category for position ' . $currentPlace . ' but got ' . $foundCategory); } } |
我們重新命名了第一個測試程式以反映它所驗證的確切的事。在兩個測試中我們移除了類別的驗證。我們知道我們有兩個不同的類別和兩個位置。根據我們所知道的和currentCategory()方法的結構,我們可以推斷將有一些包含各種類別的地方。首先我們在陣列中定義位置,接著我們期待兩個類別有兩個不同的值。
此刻,我們的目標不是測試currentCategory()方法。我們可以停止當前程式給所有位置和類別的組合寫測試程式。但我還不想那麼做。現在,我們必須仍然關注roll方法和我們的小段程式碼。我們還能移除兩個測試程式之間的重複並且抽取驗證到一個私有方法。這將對我們將來寫currentCategory()測試有幫助。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
function testRockCategoryCanBeDetermined() { $currentPlaces = [3]; $expectedCategory = 'Rock'; $this->assertCorrectCategoryForGivenPlaces($expectedCategory, $currentPlaces); } function testScienceCategoryCanBeDetermined() { $currentPlaces = [1]; $expectedCategory = 'Science'; $this->assertCorrectCategoryForGivenPlaces($expectedCategory, $currentPlaces); } |
重構roll()的第二部分
既然我們所有的測試程式已經出色地寫好並通過了,修改原始碼就是下一個邏輯步驟。包含玩家移動邏輯的if表示式必須首當其衝。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
} else { $this->movePlayer($boardSize, $rolledNumber); echoln($this->players[$this->currentPlayer] . "'s new location is " . $this->places[$this->currentPlayer]); echoln("The category is " . $this->currentCategory()); $this->askQuestion(); } |
看起來我們的方法至少需要兩個引數。告示板大小和骰子點數。餘下的使用資訊來自類變數,所以它們不需要作為引數傳遞。然而,告示板大小看起來也像是屬於方法的一個值,而不是屬於game類的。稍後,我們將看看是否可以將其移到方法內。
1 2 3 4 5 6 7 8 9 10 11 |
} else { $this->movePlayer($boardSize, $rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } |
接下來,程式碼行顯示的是使用者必須進到他們自己的小的具體方法中。我們本可以將所有的顯示物放入一個方法中,但是會很難命名它。最好是有更好的命名和更小的方法。
1 2 3 4 5 6 7 8 9 10 11 |
private function displayPlayersNewLocation() { echoln($this->players[$this->currentPlayer] . "'s new location is " . $this->places[$this->currentPlayer]); } private function displayCurrentCategory() { echoln("The category is " . $this->currentCategory()); } |
這麼做,我麼就完成了這部分程式碼。但剩下的roll()方法部分怎麼辦?
兩個開發者,兩個不同的執行緒
直到現在,這篇教程關注的重點是小的程式碼段。我們的縮放級別很接近這段程式碼。我們做了很多事去思考8到10行以內的程式碼。我們專注於變數名或者移動一到兩行程式碼從一處到另一處這樣的小事。
我們在Syneto做許多結對程式設計。基本上每次至少有一箇中等難度的任務要去完成時,我們就結對。重構是相當難的任務,所以大部分時間都有兩雙眼睛盯著程式碼。這樣允許我們通過在兩個不同的縮放級別上觀察程式碼來理解和思考我們在做的事。當一個開發者做微小的修改並且專注於字元或者行級別的細節時,另一個人可以遠處觀察程式碼。
雖然這些網頁的格式並不允許大量橫向空間,但在現實中,任何開發者都應該至少有個25寸的顯示器,這樣是允許一個更高的縮放級別的程式碼檢視的合適的解決方案。這裡是從我的27寸顯示器上看到的roll()方法。
在這個級別上,一個人可以觀察形式,縮排和替換。
這個人可以思考複雜性,程式碼設計和可以被抽出的可能方法。這個人可以評估邏輯的複雜性,另一個人同時做了微笑的修改來處理程式碼的複雜性。更高的試圖也可以相當有效得突出顯示重複。
這不是很酷嗎?在這張圖之前你看過如此巨大的重複嗎?你可以同時專注於微小細節和高階別檢視嗎?也許你是這樣。一些人有天賦去理解程式碼,但我們大多數人不能有效專注於多個層次。
它最好的部分是什麼?你也可以單獨這麼做!就是說,如果你沒有機會與另一個開發者結對程式設計,你仍然可以放大和縮小。但你必須按順序去做以便有效率。像這樣的事情就是我們在開始做的。我們在一個更高的層次上觀察方法,我們尋找可以擊破的小段程式碼並且放大它。關注點的改變有效地轉換了我們的心態和思考的方式。我們保持著放大的狀態,直到我們對它完成重構之前並不去想剩下的程式碼。現在我們可以再次縮小它,再次轉換我們的心態並且繼續了。
重構roll()的第一部分
有些人在不到一分鐘的時間裡很容易得變換了心態和觀點。另一些人需要更多的時間來“忘記”細節並且抽身出來,或者相反,放棄思考形式並開始一次閱讀一整章程式碼。如果你需要10到15分鐘,並不如你想的那麼罕見。試著以允許你把下一個十分鐘用來打破縮放界限這樣的方式來組織你的工作。
並且在此,轉換心態,我們需要放大if表示式的第一部分,當使用者在禁區的情況。
這段程式碼以另一個if()表示式開始,檢查骰子數是否是奇數。如果它是,它就做些複雜的事情。如果它是偶數,它就做些相當簡單的事。只是將玩家保持在禁區。
1 2 3 4 5 6 7 |
} else { echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box"); $this->isGettingOutOfPenaltyBox = false; } |
那將很容易測試並且讓我們專注於定義設定我們SUT的方式上。
1 2 3 4 5 6 7 8 9 10 11 |
function testAPlayerWhoIsPenalizedAndRollsAnEvenNumberWillStayInThePenaltyBox() { $rolledNumber = 2; $this->setAPlayerThatIsInThePenaltyBox(); $this->game->roll($rolledNumber); $this->assertFalse($this->game->isGettingOutOfPenaltyBox); } |
是的,很容易。漂亮的四行測試方法。並且setAPlayerThatIsInThePenaltyBox()方法和它的對手很相似。只有禁區狀態不同。
現在當骰子數是奇數時,我們可以開始為第一部分額if構建一個測試,或者幾個測試程式。
1 2 3 4 5 6 7 8 9 10 11 |
function testAPlayerWhoIsPenalizedAndRollsAnOddNumberWillGetOutOfThePenaltyBox() { $rolledNumber = 1; $this->setAPlayerThatIsInThePenaltyBox(); $this->game->roll($rolledNumber); $this->assertTrue($this->game->isGettingOutOfPenaltyBox); } |
這是有希望的開始。第一行:測試了。剩下來的將和第一層的if的else部分的測試程式相似。
結對應該直到最後
在Syneto我和我的同事面臨的一個困境是,當我們結對程式設計或者重構像這樣的事情時,當任務變得清晰並且即將完成時,開發者中的一員就想要離開了。
如果你是那個專注於寫測試程式和移動小段程式碼的人,你的同伴可能會想他的角色已經完成了。他找到高層次的問題,將它們傳達給你,現在就輪到你來修正這些程式碼了。當他試著離開時,阻止他。告訴他所有的任務開始於結對也該結束於結對。告訴他讓他幫你思考測試程式,命名,結構,以便你能夠專注於過程和步驟等需要用到的重構技術,就是那些我們在之前教程中學到的步驟。
舉例來說,他可以考慮測試命名,而你忙於複製/黏貼和修改前一個測試,迫使進入這個if部分。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 |
function testPlayerGettingOutOfPenaltyNextPositionWithoutNewLap() { $currentPlace = 2; $numberRequiredToGetOutOfPenaltyBox = 1; $this->setAPlayerThatIsInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $this->game->roll($numberRequiredToGetOutOfPenaltyBox); $this->assertEquals('3', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); } function testPlayerGettingOutOfPenaltyNextPositionWithNewLap() { $currentPlace = 11; $numberRequiredToGetOutOfPenaltyBox = 3; $this->setAPlayerThatIsInThePenaltyBox(); $this->setCurrentPlayersPosition($currentPlace); $this->game->roll($numberRequiredToGetOutOfPenaltyBox); $this->assertEquals('2', $this->getCurrentPlayersPosition(), 'Player was expected at position 3'); } |
另一方面,如果你的同伴決定在發現所有的重複和厲害的高層次問題之後,他應該有權去寫測試程式和重構程式碼,你可能會覺得你沒什麼其他事可做了。但你錯了。你可以留下並在命名,低層次的重複和其他小事上幫助他。你也可以在他磕磕絆絆或者忽略了一些小的步驟或者當他因為一個愚蠢的小錯字導致測試失敗而受阻時幫助他。
這就是測試程式如何被很好命名的,就像testPlayerGettingOutOfPenaltyNextPositionWithNewLap()和變數將表達對於當前測試程式它們所代表的含義,而不是你拷貝的之前的測試程式鎖做的:$numberRequiredToGetOutOfPenaltyBox。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 |
function roll($rolledNumber) { echoln($this->players[$this->currentPlayer] . " is the current player"); echoln("They have rolled a " . $rolledNumber); $boardSize = 12; if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; $this->movePlayer($boardSize,$rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } else { echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box"); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($boardSize, $rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } } |
這不比之前更好嗎?所有的單元測試都通過了。但我感覺我們已經移動了許多用於演示的程式碼。也許我們應該執行金牌大師測試程式了?
回退一步
1 2 3 4 5 6 7 |
PHPUnit_Framework_ExpectationFailedException : Failed asserting that false is true. Time: 18.93 seconds, Memory: 112.50Mb FAILURES! Tests: 20, Assertions: 33, Failures: 1, Skipped: 1. |
它失敗了。自我們執行金牌大師測試程式已經過去很長時間了,但我們所有的修改都在roll()方法上。所以會發生的最糟糕的事是我們恢復我們的修改。
讓我們開始往回一小步。我懷疑在我們看到的輸出中的重複,有一個小的差異。也許是我們沒觀察到的一個字母或者一個空格。我們可以恢復roll()的第一部分的輸出看看它是否起效。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 |
function roll($rolledNumber) { echoln($this->players[$this->currentPlayer] . " is the current player"); echoln("They have rolled a " . $rolledNumber); $boardSize = 12; if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; $this->movePlayer($boardSize,$rolledNumber); echoln($this->players[$this->currentPlayer] . "'s new location is " . $this->places[$this->currentPlayer]); echoln("The category is " . $this->currentCategory()); $this->askQuestion(); } else { echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box"); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($boardSize, $rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } } |
仍然失敗了。正如我們的第一個疑慮是錯誤的,我們可能想要回退一大步。金牌大師在我們開始這個重構之前是通過的嗎?也許下次我們應該首先執行它。現在我們需要把我們的修改放在一個安全地地方並且恢復所有的程式碼以驗證我們的假設。
對roll()最初狀態的恢復讓金牌大師通過了。很高興知道這點。那麼我們破壞它了。但什麼時候?什麼地方?
既然我們的程式碼恢復到原始狀態,我們可以觀察輸出,將其放到文字檔案並且將它和重構的那個進行比較。
正如我們可以立即觀察到的,在重構版本中我們遺漏了一些行。字串告訴我們玩家出禁區的部分遺漏了。嗯…
讓我們再看看我們開始的程式碼。啊!!!在那兒!
1 2 3 4 5 6 7 8 9 |
echoln($this->players[$this->currentPlayer] . " is getting out of the penalty box"); $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $roll; if ($this->playerShouldStartANewLap()) { $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize; } |
echoln()停留在移動邏輯的頂部了。一個直率的,簡單的錯誤。我們沒有注意它而只是將所有程式碼塊替換為方法呼叫。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 |
function roll($rolledNumber) { echoln($this->players[$this->currentPlayer] . " is the current player"); echoln("They have rolled a " . $rolledNumber); $boardSize = 12; if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; echoln($this->players[$this->currentPlayer] . " is getting out of the penalty box"); $this->movePlayer($boardSize,$rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } else { echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box"); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($boardSize, $rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } } |
這使得所有的測試程式通過了。謝天謝地我有個同伴幫助我發現這個問題。雖然我的同伴是一隻泰迪熊,當我獨自寫這些文章的時候,許多時候它只是幫助告訴其他人你的問題。它將是你的頭腦重演所有的思緒並且重造過程。還不止這樣,這使你意識到愚蠢的錯誤並且發現你可能錯過的事情。
新增最後的修改
在我們總結這篇教程之前,我們應該確保roll()方法已經是最佳狀態。首先,所有的echoln()呼叫都放進私有方法中。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 |
function roll($rolledNumber) { $this->displayCurrentPlayer(); $this->displayRolledNumber($rolledNumber); $boardSize = 12; if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; $this->displayPlayerGettingOutOfPenaltyBox(); $this->movePlayer($boardSize,$rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } else { $this->displayPlayerStaysInPenaltyBox(); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($boardSize, $rolledNumber); $this->displayPlayersNewLocation(); $this->displayCurrentCategory(); $this->askQuestion(); } } |
以上是在正確的方向上邁出的一步,但我的同伴說我們做得更好。
我們可以將連續的顯示功能分組到其他的顯示功能中。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 |
function roll($rolledNumber) { $this->displayStatusAfterRoll($rolledNumber); $boardSize = 12; if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; $this->movePlayer($boardSize,$rolledNumber); $this->displayStatusAfterPlayerGettingOutOfPenaltyBox(); $this->askQuestion(); } else { $this->displayPlayerStaysInPenaltyBox(); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($boardSize, $rolledNumber); $this->displayStatusAfterNonPenalizedPlayerMove(); $this->askQuestion(); } } |
這不是更好嗎?我們的方法可以只通過一個顯示呼叫跟蹤每條路徑。
你還記得$boardSize嗎?我們現在可以把它放到movePlayer()裡面了嗎?是的,我們可以。那麼,讓我們做吧。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 |
function roll($rolledNumber) { $this->displayStatusAfterRoll($rolledNumber); if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->isGettingOutOfPenaltyBox = true; $this->movePlayer($rolledNumber); $this->displayStatusAfterPlayerGettingOutOfPenaltyBox(); $this->askQuestion(); } else { $this->displayPlayerStaysInPenaltyBox(); $this->isGettingOutOfPenaltyBox = false; } } else { $this->movePlayer($rolledNumber); $this->displayStatusAfterNonPenalizedPlayerMove(); $this->askQuestion(); } } |
我們的程式碼變得相當短小了。但仍然,方法還有18行之長。那是很多的。你還記得Robert C. Martin的教程或者“魔術數字7加減2”嗎?我們的程式如果只包含四行程式碼那將更好。
往這個方向的第一步,是對於每一個可能的路徑,將其減少為對單一的方法呼叫。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 |
function roll($rolledNumber) { $this->displayStatusAfterRoll($rolledNumber); if ($this->inPenaltyBox[$this->currentPlayer]) { if ($this->isOdd($rolledNumber)) { $this->getPlayerOutOfPenaltyBoxAndPlayNextMove($rolledNumber); } else { $this->keepPlayerInPenaltyBox(); } } else { $this->playNextMove($rolledNumber); } } |
我們現在減到12行程式碼了。但我們可以做得更好。最裡面的if可以到它自己的方法中。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
function roll($rolledNumber) { $this->displayStatusAfterRoll($rolledNumber); if ($this->inPenaltyBox[$this->currentPlayer]) { $this->playNextMoveForPlayerInPenaltyBox($rolledNumber); } else { $this->playNextMove($rolledNumber); } } |
我們完成了!
這麼做我們在方法中將程式碼降低了7行。方法中只有5行,只有4行真正在做一些邏輯。現在這是一個合理的有樣子的方法了,在這點上,我覺得停留在這兒很好。並且,這不僅僅是個例子。這是“抽取直到你放棄”,並且是在Syneto的專案中我們大部分方法的樣子。這是個現實生活中的例子,並且你應該每天這樣結束你的程式碼。在這我們也結束這篇教程。
請繼續關注接下來的教程,我們將談談關於層並且我們將開始分割關注點。