Refactoring Legacy Code Del 3 - Komplexa Conditionals

Gammal kod. Ugly code. Komplicerad kod. Spaghetti kod. Gibberish nonsens. I två ord, Legacy Code. Det här är en serie som hjälper dig att arbeta och hantera det.

Jag tycker om att tänka på kod precis som jag tänker på prosa. Långa, kapslade, sammansatta meningar med exotiska ord är svåra att förstå. Från tid till annan behöver du en, men oftast kan du bara använda vanliga enkla ord i korta meningar. Detta är mycket sant för källkod också. Komplexa villkor är svåra att förstå. Långa metoder är som oändliga meningar.

Från prosa till kod

Här är ett "prosaiskt" exempel för att heja upp dig. Först all-in-one-meningen. Den fula.

Om temperaturen i serverns rummet ligger under fem grader och luftfuktigheten stiger över femtio procent men är under åttio och lufttrycket är stabilt, måste senior tekniker John, som har minst tre års arbetslivserfarenhet inom nätverks- och serveradministration meddelas, och han måste vakna upp på mitten av natten, klä upp sig, gå ut, ta bilen eller ringa en hytt om han inte har bil, kör till kontoret, gå in i byggnaden, starta luftkonditioneringen och vänta tills temperaturen ökar över tio grader och fuktigheten faller under tjugo procent.

Om du kan förstå, förstå och komma ihåg det stycket utan att läsa om det, ger jag dig en medalj (virtuellt självklart). Långa, intrasslade stycken skrivna i en enda komplicerad mening är svåra att förstå. Tyvärr vet jag inte tillräckligt exotiska engelska ord för att göra det ännu svårare att förstå.

Förenkling

Låt oss hitta ett sätt att förenkla det lite. Hela sin första del, tills "då" är ett villkor. Ja, det är komplicerat men vi kan summera det så här: Om miljöförhållandena utgör en risk ... ... då bör något göras. Det komplicerade uttrycket säger att vi bör anmäla någon som uppfyller många villkor: sedan meddela nivå tre tech support. Slutligen beskrivs en hel process från att vakna tech killen tills allt är löst: och förvänta dig att miljön ska återställas inom normala parametrar. Låt oss lägga allt ihop.

Om miljöförhållandena utgör en risk, anmäla nivå tre tekniskt stöd och förvänta sig att miljön ska återställas inom normala parametrar.

Nu är det bara ca 20% i längd jämfört med originaltexten. Vi känner inte till detaljerna och i de allra flesta fallen bryr vi oss inte om. Och det här är mycket sant för källkoden också. Hur många gånger bryr du dig om implementeringsdetaljerna för a logInfo ("Några meddelanden"); metod? Förmodligen en gång, om och när du implementerade den. Då loggar det bara meddelandet i "info" kategorin. Eller när en användare köper en av dina produkter bryr du dig om hur du fakturerar honom? Nej. Allt du vill bry sig om är Om produkten köptes, kassera den från lager och fakturera den till köparen. Exemplen kan vara oändliga. De är i princip hur vi skriver rätt programvara.

Komplexa kondensatorer

I det här avsnittet kommer vi att försöka tillämpa prosafilosofin på vårt trivia-spel. Ett steg i taget. Börjar med komplexa villkor. Låt oss börja med en enkel kod. Bara för att värma upp.

Linje tjugo av GameRunner.php filen läser så här:

om (rand ($ minAnswerId, $ maxAnswerId) == $ felaktigt svarat)

Hur skulle det låta i prosa? Om ett slumptal mellan minsta svar-ID och maximalt svar-ID motsvarar det felaktiga svarets ID, så ...

Det här är inte särskilt komplicerat, men vi kan fortfarande göra det enklare. Hur är detta? Om fel svar är valt, så ... Bättre är det inte?

Extraktmetoden Refactoring

Vi behöver ett sätt, ett förfarande, en teknik för att flytta det villkorliga uttalandet någon annanstans. Det målet kan enkelt vara en metod. Eller i vårt fall, eftersom vi inte är inne i en klass här, en funktion. Denna flyttning av beteende till en ny metod eller funktion kallas refraktoreringsmetoden "Extract Method". Nedan följer stegen, som definieras av Martin Fowler i sin utmärkta bok Refactoring: Förbättra designen av befintlig kod. Om du inte läste den här boken bör du lägga den på din "Att läsa" -lista nu. Det är en av de mest nödvändiga böckerna för en modernt programmerare.

För vår handledning har jag tagit de ursprungliga stegen och förenklat dem lite för att bättre passa våra behov och vår typ av handledning.

  1. Skapa en ny metod och namnge den efter vad den gör, inte hur den gör det.
  2. Kopiera koden från den extraherade platsen till metoden. Observera att detta är kopia, Ta inte bort originalkoden ännu.
  3. Skanna den extraherade koden för alla variabler som är lokala. De måste göras parametrar för metoden.
  4. Se om några tillfälliga variabler används inom den extraherade metoden. Om så är fallet, deklarera dem inuti det och släpp den extra parametern.
  5. Passa in i målmetoden variablerna som parametrar.
  6. Byt ut den extraherade koden med samtalet till målmetoden.
  7. Kör dina tester.

Nu är det ganska komplicerat. Emellertid är extraktmetoden förmodligen den mest använda refaktorn, förutom att omdöpa kanske. Så du måste förstå dess mekanik.

Lyckligtvis för oss, idag erbjuder IDEs som PHPStorm bra refactoringverktyg, som vi har sett i handledningen PHPStorm: När IDE verkligen gäller. Så vi kommer att använda de funktioner vi har vid våra fingertoppar istället för att göra allt för hand. Detta är mindre felaktigt och mycket, mycket snabbare.

Välj bara önskad del av koden och Högerklicka Det.

IDE kommer automatiskt att förstå att vi behöver tre parametrar för att köra vår kod och den kommer att föreslå följande lösning.

// ... // $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; funktionen ärCurrentAnswerWrong ($ minAnswerId, $ maxAnswerId, $ wrongAnswerId) return rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId;  gör $ tärning = rand (0, 5) + 1; $ AGame-> rulle ($ tärningar); om (isCurrentAnswerWrong ($ minAnswerId, $ maxAnswerId, $ wrongAnswerId)) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  medan ($ notAWinner);

Medan denna kod är syntaktiskt korrekt kommer den att bryta våra test. Mellan alla de ljud som visas i oss i röda, blå och svarta färger kan vi upptäcka anledningen till varför:

Felaktigt fel: Kan inte redeclare isCurrentAnswerWrong () (tidigare angiven i / home / csaba / Personal / Programming / NetTuts / Refactoring Legacy Code - Del 3: Komplexa villkor och långa metoder / Source / trivia /php/GameRunner.php:16) in / hem / csaba / Personal / Programmering / NetTuts / Refactoring Legacy Code - Del 3: Komplexa villkor och långa metoder / Source / Trivia /php/GameRunner.php på rad 18

Det säger i princip att vi vill deklarera funktionen två gånger. Men hur kan det hända? Vi har det bara en gång i vårt GameRunner.php!

Ta en titt på testerna. Det finns en generateOutput () metod som gör en fordra() på vår GameRunner.php. Det kallas minst två gånger. Här är källan till felet.

Nu har vi ett dilemma. På grund av den slumpmässiga generatorns sådd behöver vi kalla denna kod med kontrollerade värden.

privat funktion genereraOutput ($ seed) ob_start (); srand ($ utsäde); kräver __DIR__. '/ ... /trivia/php/GameRunner.php'; $ output = ob_get_contents (); ob_end_clean (); returnera $ output; 

Men det finns inget sätt att deklarera en funktion två gånger i PHP, så vi behöver en annan lösning. Vi börjar känna byrden av vår gyllene mästertestning. Att köra hela satsen 20000 gånger, kan vi inte vara en lösning på lång sikt varje gång vi byter kod. Förutom det faktum att det tar åldrar att springa, tvingar det oss att ändra vår kod för att rymma hur vi testar det. Detta är vanligtvis ett tecken på dåliga test. Koden ska ändras och göra testet fortfarande, men ändringarna borde ha skäl att ändra, som bara kommer från källkoden.

Men tillräckligt tal, vi behöver en lösning, även en tillfällig kommer att göra för nu. Migrering till enhetsprov börjar med vår nästa lektion.

Ett sätt att lösa vårt problem är att ta hela resten av koden in GameRunner.php och lägg den i en funktion. Låt oss säga springa()

include_once __DIR__. '/Game.php'; funktionen ärCurrentAnswerWrong ($ minAnswerId, $ maxAnswerId, $ wrongAnswerId) return rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId;  funktionskörning () $ notAWinner; $ aGame = nytt spel (); $ AGame-> lägga ( "Chet"); $ AGame-> lägga ( "Pat"); $ AGame-> lägga ( "Sue"); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; gör $ tärning = rand (0, 5) + 1; $ AGame-> rulle ($ tärningar); om (isCurrentAnswerWrong ($ minAnswerId, $ maxAnswerId, $ wrongAnswerId)) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  medan ($ notAWinner); 

Detta gör det möjligt för oss att testa det, men var medveten om att köra koden från konsolen inte kommer att köra spelet. Vi gjorde en liten förändring i beteendet. Vi fick testbarhet till kostnaden för en beteendeförändring, som vi inte ville göra i första hand. Om du vill köra koden från konsolen behöver du nu en annan PHP-fil som innehåller eller kräver löparen och kallar då explicit körmetoden på den. Inte så stor förändring utan ett måste att komma ihåg, speciellt om du har tredje part som använder din befintliga kod.

Å andra sidan kan vi nu bara inkludera filen i vårt test.

kräver __DIR__. '/ ... /trivia/php/GameRunner.php';

Och ring då springa() inuti metoden generateOutput ().

privat funktion genereraOutput ($ seed) ob_start (); srand ($ utsäde); springa(); $ output = ob_get_contents (); ob_end_clean (); returnera $ output; 

Katalogstruktur, filer, och namngivning

Kanske är det här ett bra tillfälle att tänka på strukturen i våra kataloger och filer. Det finns inga mer komplexa villkor i vår GameRunner.php, men innan vi fortsätter till Game.php fil, vi får inte lämna en röra bakom oss. Vår GameRunner.php kör inte längre, och vi behövde hacka metoder tillsammans för att göra det testbart vilket bröt vårt offentliga gränssnitt. Anledningen till detta är att vi kanske testar fel sak.

Vårt test samtal springa() i GameRunner.php fil som innehåller Game.php, spelar spelet och en ny gyllene huvudfil genereras. Vad händer om vi presenterar en annan fil? Vi gör GameRunner.php att faktiskt köra spelet genom att ringa springa() och ingenting annat. Så vad händer om det inte finns någon logik där kan det gå fel och inga test behövs, och då flyttar vi vår nuvarande kod till en annan fil?

Nu är det här en helt annan historia. Nu har våra tester tillgång till koden precis under löparen. I grund och botten är våra test bara löpare. Och naturligtvis i vårt nya GameRunner.php det kommer bara att finnas ett samtal för att köra spelet. Detta är en sann löpare, det gör inget annat än att kalla springa() metod. Ingen logik innebär att inga test behövs.

require_once __DIR__. '/RunnerFunctions.php'; springa();

Det finns andra frågor som vi kan fråga oss själva vid denna tidpunkt. Behöver vi verkligen en RunnerFunctions.php? Kunde vi inte bara ta funktionerna därifrån och flytta dem till Game.php? Vi kan förmodligen, men med vår nuvarande förståelse av, vilken funktion tillhör där? Är inte tillräckligt. Vi kommer att hitta en plats för vår metod i en kommande lektion.

Vi försökte också namnge våra filer enligt vad koden inuti dem gör. En är bara en massa funktioner för löparen, funktioner vi, vid denna tidpunkt anser att vi tillhör varandra för att tillgodose behoven hos löparen. Kommer det att bli en klass på en tidpunkt i framtiden? Kanske. Kanske inte. För nu är det tillräckligt bra.

Rengöring av RunnerFunctions

Om vi ​​tittar på RunnerFunctions.php fil, det finns en liten röra som vi har introducerat.

Vi definierar:

$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7;

… inuti springa() metod. De har en enda anledning att existera och en enda plats där de används. Varför inte bara definiera dem inom den metoden och bli av med parametrarna helt och hållet?

funktionen ärCurrentAnswerWrong () $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; returnera rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId; 

Ok, testerna passerar och koden är mycket trevligare. Men inte tillräckligt bra.

Negativa Conditionals

Det är mycket lättare, för det mänskliga sinnet, att förstå positiv resonemang. Så om du kan undvika negativa villkor måste du alltid ta den vägen. I vårt nuvarande exempel kontrollerar metoden ett felaktigt svar. Det skulle vara mycket lättare att förstå en metod som kontrollerar en validitet och negerar det när det behövs.

funktionen ärCurrentAnswerCorrect () $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; returnera rand ($ minAnswerId, $ maxAnswerId)! = $ wrongAnswerId; 

Vi använde reflekteringsmetoden Rename Method. Detta är igen, ganska komplicerat om det används för hand, men i någon IDE är det lika enkelt som att slå CTRL + r, eller välja rätt alternativ i menyn. För att våra test ska passera behöver vi också uppdatera vårt villkorade uttalande med en negation.

om (! isCurrentAnswerCorrect ()) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); 

Detta ger oss ett steg närmare vår förståelse av de villkorliga. Använder sig av ! i en om() uttalande, hjälper verkligen. Det sticker ut och belyser det faktum att någonting faktiskt är negativt där. Men kan vi vända om detta för att undvika negation helt? Ja det kan vi.

om (isCurrentAnswerCorrect ()) $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  annars $ notAWinner = $ aGame-> wrongAnswer (); 

Nu har vi ingen logisk negation genom att använda !, inte heller lexisk negation genom att namnge och returnera de felaktiga sakerna. Alla dessa steg gjorde vårt villkorade mycket, mycket lättare att förstå.

Conditionals i Game.php

Vi förenklade till det yttersta, RunnerFunctions.php. Låt oss attackera vårt Game.php filen nu. Det finns flera sätt att söka efter conditionals. Om du föredrar kan du bara skanna koden genom att bara titta på den. Detta är långsammare, men har mervärdet att tvinga dig att försöka förstå det i följd.

Det andra uppenbara sättet att söka efter conditionals är att bara hitta en "if" eller "if (". Om du formaterade din kod med de inbyggda funktionerna i din IDE kan du vara säker på att alla villkorliga uttalanden har Samma specifika form. I mitt fall finns det ett mellanslag mellan "if" och parentes. Om du använder den inbyggda sökningen kommer de hittade resultaten att markeras i en stark färg, i mina fall gul.

Nu när vi alla har upplyst vår kod som ett julgran kan vi ta dem en efter en. Vi känner till borren, vi känner till de tekniker vi kan använda, det är dags att tillämpa dem.

om ($ this-> inPenaltyBox [$ this-> currentPlayer])

Det verkar ganska rimligt. Vi kunde extrahera det till en metod, men skulle det finnas ett namn på den metoden för att göra villkoret tydligare?

om ($ roll% 2! = 0) 

Jag satsar 90% av alla programmerare kan förstå problemet i ovanstående om påstående. Vi försöker koncentrera oss på vad vår nuvarande metod gör. Och vår hjärna är ansluten till problemets domän. Vi vill inte "starta en annan tråd" för att beräkna det matematiska uttrycket för att förstå att det bara kontrollerar om ett tal är udda. Detta är en av de små distraktioner som kan förstöra ett svårt logiskt avdrag. Så jag säger att vi ska ta ut det.

om ($ this-> isOdd ($ roll))

Det är bättre eftersom det handlar om problemets domän och kräver ingen extra hjärnkraft.

om ($ this-> placerar [$ this-> currentPlayer]> $ lastPositionOnTheBoard)

Detta verkar vara en annan bra kandidat. Det är inte så svårt att förstå som ett matematiskt uttryck, men återigen är det ett uttryck som behöver sidbehandling. Jag frågar mig själv, vad betyder det om den nuvarande spelarens position nått slutet av styrelsen? Kan vi inte uttrycka detta tillstånd på ett mer konkret sätt? Vi kan förmodligen.

om ($ this-> playerReachedEndOfBoard ($ lastPositionOnTheBoard))

Detta är bättre. Men vad händer faktiskt inuti om? Spelaren är omplacerad i början av styrelsen. Spelaren startar ett nytt "varv" i loppet. Vad händer om vi i framtiden kommer att ha en annan anledning till att starta ett nytt varv? Ska vårt om uttalande förändras när vi ändrar den underliggande logiken i den privata metoden? Absolut inte! Så, låt oss byta namn på den här metoden i vad om representerar, i vad som händer, inte vad vi söker efter.

om ($ this-> playerShouldStartANewLap ($ lastPositionOnTheBoard))

När du försöker namnge metoder och variabler, tänk alltid på vad koden ska göra och inte vilken stat eller skick det representerar. När du får det här, kommer omdirigering av åtgärder i din kod att minska avsevärt. Men fortfarande, även en erfaren programmerare behöver byta namn på en metod minst tre till fem gånger innan man hittar sitt korrekta namn. Så var inte rädd för att träffa CTRL + r och byt namn på ofta. Förplikta aldrig dina ändringar i projektets VCS om du inte skannat namnen på dina nyligen tillagda metoder och din kod läser inte som välskrivna prosa. Byte namn är så billigt i våra dagar att du kan byta namn på saker bara för att prova olika versioner och återgå med en enda knapptryckning.

De om uttalandet vid rad 90 är detsamma som vår tidigare. Vi kan bara återanvända vår extraherade metod. Voila, dubbelarbete elimineras! Och glöm inte att köra dina tester nu och då, även när du refactor använder din IDEs magi. Vilket leder oss till vår nästa observation. Magiska, ibland, misslyckas. Kolla in rad 65.

$ lastPositionOnTheBoard = 11;

Vi förklarar en variabel och använder den bara på en enda plats, som en parameter till vår nyligen extraherade metod. Detta föreslår starkt att variabeln borde vara inuti metoden.

privat funktion playerShouldStartANewLap () $ lastPositionOnTheBoard = 11; returnera $ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard; 

Och glöm inte att ringa metoden utan några parametrar i din om uttalanden.

om ($ this-> playerShouldStartANewLap ())

De om uttalanden i frågar frågan() Metoden verkar vara bra, liksom de som finns i currentCategory ().

om ($ this-> inPenaltyBox [$ this-> currentPlayer])

Detta är lite mer komplicerat, men i domänen och uttrycksfullt nog.

om ($ this-> currentPlayer == count ($ this-> spelare))

Vi kan arbeta med den här. Det är uppenbart att jämförelsen betyder att den nuvarande spelaren är obestämd. Men som vi lärde oss ovan vill vi att intentionen inte är statligt.

om ($ this-> shouldResetCurrentPlayer ())

Det är mycket bättre, och vi kommer att återanvända det på rad 172, 189 och 203. Dubbling, jag menar triplication, jag menar fyrdubbling, elimineras!

Tester passerar och allt om uttalanden utvärderades för komplexitet.

Slutgiltiga tankar

Det finns flera lektioner som kan läras av refactoring villkor. Först och främst hjälper de bättre att förstå meningen med koden. Då om du namnger den extraherade metoden för att representera avsiktet korrekt, kommer du att undvika framtida namnändringar. Att hitta duplicering i logiken är svårare än att hitta dubbla linjer med enkel kod. Du kanske har trott att vi borde göra en medveten dubbelarbete, men jag föredrar att hantera dubbelarbete när jag har enhetstester som jag kan lita på mitt liv med. Den gyllene mästaren är bra, men det är högst ett säkerhetsnät, inte en fallskärm.

Tack för att du läste och håll dig inställd på vår nästa handledning när vi introducerar våra första testsatser.