1. Code
  2. Coding Fundamentals
  3. Design Patterns

Refactoring Legacy Code: Teil 2 -  Magische Saiten &  Konstanten

Alter Code. Hässlicher Code. Komplizierter Code. Spaghetti-Code. Quatsch Unsinn. In zwei Worten, Legacy Code. Das ist eine Serie, die Ihnen hilft, damit zu arbeiten und umzugehen.
Scroll to top
This post is part of a series called Refactoring Legacy Code.
Refactoring Legacy Code: Part 1 - The Golden Master
Refactoring Legacy Code: Part 3 - Complex Conditionals

German (Deutsch) translation by Katharina Grigorovich-Nevolina (you can also view the original English article)

Alter Code. Hässlicher Code. Komplizierter Code. Spaghetti-Code. Quatsch Unsinn. In zwei Worten, Legacy Code. Das ist eine Serie, die Ihnen hilft, damit zu arbeiten und umzugehen.

Wir haben unseren alten Quellcode zum ersten Mal in unserer vorherigen Lektion kennengelernt. Anschließend haben wir den Code ausgeführt, um eine Meinung zu seiner Grundfunktionalität zu bilden, und eine Golden Master-Testsuite erstellt, um ein Sicherheitsnetz für zukünftige Änderungen zu erhalten. Wir werden diesen Code weiter bearbeiten und Sie finden ihn im Ordner trivia/php_start. Der andere Ordner trivia/php_start enthält den fertigen Code dieser Lektion.

Die Zeit für die ersten Änderungen ist gekommen und wie kann man eine schwierige Codebasis besser verstehen, als magische Konstanten und Strings in Variablen zu extrahieren? Diese scheinbar einfachen Aufgaben geben uns größere und manchmal unerwartete Einblicke in das Innenleben von Legacy-Code. Wir müssen die Absichten des ursprünglichen Code-Autors herausfinden und die richtigen Namen für die Code-Teile finden, die wir noch nie gesehen haben.

Magische Saiten

Magische Zeichenfolgen sind Zeichenfolgen, die direkt in verschiedenen Ausdrücken verwendet werden, ohne einer Variablen zugewiesen zu sein. Diese Art von Zeichenfolge hatte eine besondere Bedeutung für den ursprünglichen Autor des Codes, aber anstatt sie einer gut benannten Variablen zuzuweisen, hielt der Autor die Bedeutung der Zeichenfolge für offensichtlich genug.

Identifizieren Sie die ersten zu ändernden Zeichenfolgen

Schauen wir uns zunächst unsere Game.php an und versuchen, Strings zu identifizieren. Wenn Sie eine IDE verwenden (und sollten) oder einen intelligenteren Texteditor, der Quellcode hervorheben kann, ist das Erkennen der Zeichenfolgen einfach. Hier ist ein Bild davon, wie der Code auf meinem Display aussieht.

Alles mit Orange ist eine Schnur. Auf diese Weise ist es sehr einfach, jede Zeichenfolge in unserem Quellcode zu finden. Stellen Sie daher sicher, dass die Hervorhebung in Ihrem Editor unterstützt und aktiviert wird, unabhängig von Ihrer Anwendung.

Der erste orangefarbene Teil in unserem Code befindet sich sofort in Zeile drei. Die Zeichenfolge enthält jedoch nur ein Zeilenumbruchzeichen. Das sollte meiner Meinung nach offensichtlich genug sein, damit wir weitermachen können.

Wenn es darum geht, zu entscheiden, was extrahiert und was unverändert bleiben soll, gibt es nur wenige Daumen hoch, aber am Ende muss sich Ihre berufliche Meinung durchsetzen. Auf dieser Grundlage müssen Sie entscheiden, was mit jedem von Ihnen analysierten Code geschehen soll.

1
for ($i = 0; $i < 50; $i++) {
2
		array_push($this->popQuestions, "Pop Question " . $i);
3
		array_push($this->scienceQuestions, ("Science Question " . $i));
4
		array_push($this->sportsQuestions, ("Sports Question " . $i));
5
		array_push($this->rockQuestions, $this->createRockQuestion($i));
6
	}
7
}
8
9
function createRockQuestion($index) {
10
	return "Rock Question " . $index;
11
}

Analysieren wir also die Zeilen 32 bis 42, den Ausschnitt, den Sie oben sehen können. Für Pop-, Wissenschafts- und Sportfragen gibt es nur eine einfache Verkettung. Die Aktion zum Erstellen der Zeichenfolge für eine Rock-Frage wird jedoch in eine Methode extrahiert. Sind diese Verkettungen und Zeichenfolgen Ihrer Meinung nach klar genug, damit wir sie alle in unserer for-Schleife behalten können? Oder glauben Sie, dass das Extrahieren aller Zeichenfolgen in ihre Methoden die Existenz dieser Methoden rechtfertigen würde? Wenn ja, wie würden Sie diese Methoden benennen?

Aktualisieren Sie Golden Master und die Tests

Unabhängig von der Antwort müssen wir den Code ändern. Es ist Zeit, unseren Goldenen Meister zum Arbeiten zu bringen und unseren Test zu schreiben, der unseren Code tatsächlich ausführt und mit dem vorhandenen Inhalt vergleicht.

1
class GoldenMasterTest extends PHPUnit_Framework_TestCase {
2
3
	private $gmPath;
4
5
	function setUp() {
6
		$this->gmPath = __DIR__ . '/gm.txt';
7
	}
8
9
	function testGenerateOutput() {
10
		$times = 20000;
11
		$this->generateMany($times, $this->gmPath);
12
	}
13
14
	function testOutputMatchesGoldenMaster() {
15
		$times = 20000;
16
		$actualPath = '/tmp/actual.txt';
17
		$this->generateMany($times, $actualPath);
18
		$file_content_gm = file_get_contents($this->gmPath);
19
		$file_content_actual = file_get_contents($actualPath);
20
		$this->assertTrue($file_content_gm == $file_content_actual);
21
	}
22
23
	private function generateMany($times, $fileName) {...}
24
25
	private function generateOutput($seed) {...}
26
}

Wir haben einen weiteren Test zum Vergleichen der Ausgaben erstellt und sichergestellt, dass testGenerateOutput() die Ausgabe nur einmal generiert und nichts anderes tut. Wir haben auch die goldene Master-Ausgabedatei "gm.txt" in das aktuelle Verzeichnis verschoben, da "/tmp" beim Neustart des Systems möglicherweise gelöscht wird. Für unsere tatsächlichen Ergebnisse können wir es weiterhin verwenden. Auf den meisten UNIX-ähnlichen Systemen ist "/tmp" im RAM installiert, sodass es viel schneller als das Dateisystem ist. Wenn Sie es gut gemacht haben, sollten die Tests problemlos bestehen.

Es ist sehr wichtig, sich daran zu erinnern, unseren Generatortest für zukünftige Änderungen als "übersprungen" zu markieren. Wenn Sie sich beim Kommentieren oder Löschen ganz wohl fühlen, tun Sie dies bitte. Es ist wichtig, dass sich unser Goldener Meister nicht ändert, wenn wir unseren Code ändern. Es wurde einmal generiert und wir möchten es niemals ändern, damit wir sicher sein können, dass unser neu generierter Code immer mit dem Original verglichen wird. Wenn Sie sich beim Erstellen eines Backups wohler fühlen, fahren Sie bitte fort.

1
function testGenerateOutput() {
2
	$this->markTestSkipped();
3
	$times = 20000;
4
	$this->generateMany($times, $this->gmPath);
5
}

Ich werde den Test nur als übersprungen markieren. Dadurch wird unser Testergebnis auf "yellow" gesetzt, was bedeutet, dass alle Tests bestanden wurden, einige jedoch übersprungen oder als unvollständig markiert wurden.

Unsere erste Änderung vornehmen

Mit vorhandenen Tests können wir beginnen, den Code zu ändern. Meiner professionellen Meinung nach können alle Saiten in der for-Schleife gehalten werden.  Wir nehmen den Code aus der Methode createRockQuestion(), verschieben ihn in die for-Schleife und löschen die Methode vollständig. Dieses Refactoring wird als Inline-Methode bezeichnet.

"Fügen Sie den Body der Methode in den Body ihrer Aufrufer ein und entfernen Sie die Methode." ~ Martin Fowler

Es gibt eine Reihe von Schritten, um diese Art von Refactoring durchzuführen, wie von Marting Fowler in Refactoring definiert: Verbesserung des Designs vorhandenen Codes:

  • Stellen Sie sicher, dass die Methode nicht polymorph ist.
  • Finden Sie alle Aufrufe der Methode.
  • Ersetzen Sie jeden Aufruf durch den Methodenkörper.
  • Kompilieren und testen.
  • Entfernen Sie die Methodendefinition.

Wir haben keine Unterklassen, die Game erweitern, daher wird der erste Schritt bestätigt.

Es gibt nur eine einzige Verwendung unserer Methode innerhalb der for-Schleife.

1
function  __construct() {
2
3
	$this->players = array();
4
	$this->places = array(0);
5
	$this->purses = array(0);
6
	$this->inPenaltyBox = array(0);
7
8
	$this->popQuestions = array();
9
	$this->scienceQuestions = array();
10
	$this->sportsQuestions = array();
11
	$this->rockQuestions = array();
12
13
	for ($i = 0; $i < 50; $i++) {
14
		array_push($this->popQuestions, "Pop Question " . $i);
15
		array_push($this->scienceQuestions, ("Science Question " . $i));
16
		array_push($this->sportsQuestions, ("Sports Question " . $i));
17
		array_push($this->rockQuestions, "Rock Question " . $i);
18
	}
19
}
20
21
function createRockQuestion($index) {
22
	return "Rock Question " . $index;
23
}

Wir setzen den Code von createRockQuestion() in die for-Schleife im Konstruktor. Wir haben immer noch unseren alten Code. Es ist jetzt Zeit, unseren Test durchzuführen.

Unsere Tests bestehen. Wir können unsere Methode createRockQuestion() löschen.

1
function  __construct() {
2
3
	// ... //

4
5
	for ($i = 0; $i < 50; $i++) {
6
		array_push($this->popQuestions, "Pop Question " . $i);
7
		array_push($this->scienceQuestions, ("Science Question " . $i));
8
		array_push($this->sportsQuestions, ("Sports Question " . $i));
9
		array_push($this->rockQuestions, "Rock Question " . $i);
10
	}
11
}
12
13
function isPlayable() {
14
	return ($this->howManyPlayers() >= 2);
15
}

Schließlich sollten wir unsere Tests erneut ausführen. Wenn wir einen Aufruf einer Methode verpasst haben, schlagen sie fehl.

Sie sollten wieder passieren. Glückwunsch! Wir sind mit unserem ersten Refactoring fertig.

Andere Zeichenfolgen berücksichtigen 

Zeichenfolgen in den Methoden add() und roll() werden nur verwendet, um sie mit der Methode echoln() auszugeben. askQuestions() vergleicht Zeichenfolgen mit Kategorien. Das scheint auch akzeptabel. currentCategory() hingegen gibt Zeichenfolgen zurück, die auf einer Zahl basieren. Bei dieser Methode gibt es viele doppelte Zeichenfolgen. Das Ändern einer Kategorie außer Rock würde das Ändern des Namens an drei Stellen erfordern, nur bei dieser Methode.

1
function currentCategory() {
2
	if ($this->places[$this->currentPlayer] == 0) {
3
		return "Pop";
4
	}
5
	if ($this->places[$this->currentPlayer] == 4) {
6
		return "Pop";
7
	}
8
	if ($this->places[$this->currentPlayer] == 8) {
9
		return "Pop";
10
	}
11
	if ($this->places[$this->currentPlayer] == 1) {
12
		return "Science";
13
	}
14
	if ($this->places[$this->currentPlayer] == 5) {
15
		return "Science";
16
	}
17
	if ($this->places[$this->currentPlayer] == 9) {
18
		return "Science";
19
	}
20
	if ($this->places[$this->currentPlayer] == 2) {
21
		return "Sports";
22
	}
23
	if ($this->places[$this->currentPlayer] == 6) {
24
		return "Sports";
25
	}
26
	if ($this->places[$this->currentPlayer] == 10) {
27
		return "Sports";
28
	}
29
	return "Rock";
30
}

Ich denke, wir können es besser machen. Wir können eine Refactoring-Methode namens Introduce Local Variable verwenden und die Duplizierung beseitigen. Befolgen Sie diese Richtlinien:

  • Fügen Sie eine Variable mit dem gewünschten Wert hinzu.
  • Finden Sie alle Verwendungen des Wertes.
  • Ersetzen Sie alle Verwendungen durch die Variable.
1
function currentCategory() {
2
	$popCategory = "Pop";
3
	$scienceCategory = "Science";
4
	$sportCategory = "Sports";
5
	$rockCategory = "Rock";
6
7
	if ($this->places[$this->currentPlayer] == 0) {
8
		return $popCategory;
9
	}
10
	if ($this->places[$this->currentPlayer] == 4) {
11
		return $popCategory;
12
	}
13
	if ($this->places[$this->currentPlayer] == 8) {
14
		return $popCategory;
15
	}
16
	if ($this->places[$this->currentPlayer] == 1) {
17
		return $scienceCategory;
18
	}
19
	if ($this->places[$this->currentPlayer] == 5) {
20
		return $scienceCategory;
21
	}
22
	if ($this->places[$this->currentPlayer] == 9) {
23
		return $scienceCategory;
24
	}
25
	if ($this->places[$this->currentPlayer] == 2) {
26
		return $sportCategory;
27
	}
28
	if ($this->places[$this->currentPlayer] == 6) {
29
		return $sportCategory;
30
	}
31
	if ($this->places[$this->currentPlayer] == 10) {
32
		return $sportCategory;
33
	}
34
	return $rockCategory;
35
}

Das ist viel besser. Sie haben wahrscheinlich bereits einige zukünftige Verbesserungen beobachtet, die wir an unserem Code vornehmen könnten, aber wir stehen erst am Anfang unserer Arbeit. Es ist verlockend, alles, was Sie finden, sofort zu reparieren, aber bitte nicht. Oft, insbesondere bevor der Code gut verstanden wird, können verlockende Änderungen zu Sackgassen oder sogar zu mehr Codebruch führen. Wenn Sie der Meinung sind, dass Sie Ihre Idee möglicherweise vergessen, notieren Sie sie einfach auf einem Notizzettel oder erstellen Sie eine Aufgabe in Ihrer Projektmanagement-Software. Jetzt müssen wir mit unseren String-Problemen fortfahren.

Im Rest der Datei werden alle Zeichenfolgen ausgegeben und an echoln() gesendet. Wir werden sie vorerst unberührt lassen. Eine Änderung würde sich auf das Drucken und Bereitstellen der Logik unserer Anwendung auswirken. Sie sind Teil der Präsentationsschicht, gemischt mit Geschäftslogik. Wir werden uns in einer zukünftigen Lektion mit der Trennung verschiedener Anliegen befassen.

Magische Konstanten

Magische Konstanten sind magischen Strings sehr ähnlich, aber mit Werten. Diese Werte können boolesche Werte oder Zahlen sein. Wir werden uns hauptsächlich auf Zahlen konzentrieren, die in if-Anweisungen oder return-Anweisungen oder anderen Ausdrücken verwendet werden. Wenn diese Zahlen eine unklare Bedeutung haben, müssen wir sie in Variablen oder Methoden extrahieren.

1
include_once __DIR__ . '/Game.php';
2
3
$notAWinner;
4
5
$aGame = new Game();
6
7
$aGame->add("Chet");
8
$aGame->add("Pat");
9
$aGame->add("Sue");
10
11
do {
12
13
	$aGame->roll(rand(0, 5) + 1);
14
15
	if (rand(0, 9) == 7) {
16
		$notAWinner = $aGame->wrongAnswer();
17
	} else {
18
		$notAWinner = $aGame->wasCorrectlyAnswered();
19
	}
20
21
} while ($notAWinner);

Wir werden dieses Mal mit GameRunner.php beginnen und unser erster Fokus ist die roll()-Methode, die einige Zufallszahlen erhält. Dem vorherigen Autor war es nicht wichtig, diesen Zahlen eine Bedeutung zu geben. Können wir? Wenn wir den Code analysieren:

1
rand(0, 5) + 1

Es wird eine Zahl zwischen eins und sechs zurückgegeben. Der zufällige Teil gibt eine Zahl zwischen null und fünf zurück, zu der wir immer eine hinzufügen. Es ist also sicher zwischen eins und sechs. Jetzt müssen wir den Kontext unserer Anwendung betrachten. Wir entwickeln ein Quizspiel. Wir wissen, dass es eine Art Brett gibt, auf dem sich unsere Spieler bewegen müssen. Und dazu müssen wir würfeln. Ein Würfel hat sechs Gesichter und kann Zahlen zwischen eins und sechs erzeugen. Das scheint ein vernünftiger Abzug zu sein.

1
$dice = rand(0, 5) + 1;
2
$aGame->roll($dice);

Ist das nicht schön Wir haben das Refactoring-Konzept "Lokale Variable einführen" erneut verwendet. Wir haben unsere neue Variable $dice benannt und sie repräsentiert die Zufallszahl zwischen eins und sechs. Dies ließ auch unsere nächste Aussage ganz natürlich klingen: Spiel, Würfel rollen.

Haben Sie Ihre Tests durchgeführt? Ich habe es nicht erwähnt, aber wir müssen sie so oft wie möglich ausführen. Wenn Sie das nicht getan haben, wäre das ein guter Zeitpunkt, um sie auszuführen. Und sie sollten bestehen.

Es ging also nur darum, eine Zahl mit einer Variablen auszutauschen. Wir haben einen ganzen Ausdruck, der eine Zahl darstellt, in eine Variable extrahiert. Das kann technisch als Magic Constant-Fall betrachtet werden, aber nicht als reiner Fall. Was ist mit unserem nächsten zufälligen Ausdruck?

1
if (rand(0, 9) == 7)

Das ist schwieriger. Was sind Null, Neun und Sieben in diesem Ausdruck? Vielleicht können wir sie benennen. Auf den ersten Blick habe ich keine guten Ideen für null und neun, also versuchen wir es mit sieben. Wenn die von unserer Zufallsfunktion zurückgegebene Zahl gleich sieben ist, geben wir den ersten Zweig der if-Anweisung ein, der eine falsche Antwort ergibt. Vielleicht könnten unsere sieben $wrongAnswerId heißen.

1
$wrongAnswerId = 7;
2
if (rand(0, 9) == $wrongAnswerId) {
3
	$notAWinner = $aGame->wrongAnswer();
4
} else {
5
	$notAWinner = $aGame->wasCorrectlyAnswered();
6
}

Unsere Tests bestehen noch und der Code ist etwas aussagekräftiger. Nachdem wir es geschafft haben, unsere Nummer sieben zu benennen, wird die Bedingung in einen anderen Kontext gestellt. Wir können uns auch einige anständige Namen für Null und Neun vorstellen. Sie sind nur Parameter für rand(), daher werden die Variablen wahrscheinlich als min-etwas und max-etwas bezeichnet.

1
$minAnswerId = 0;
2
$maxAnswerId = 9;
3
$wrongAnswerId = 7;
4
if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
5
	$notAWinner = $aGame->wrongAnswer();
6
} else {
7
	$notAWinner = $aGame->wasCorrectlyAnswered();
8
}

Das ist ausdrucksstark. Wir haben eine minimale Antwort-ID, eine maximale und eine andere für die falsche Antwort. Geheimnis gelüftet.

1
do {
2
3
	$dice = rand(0, 5) + 1;
4
	$aGame->roll($dice);
5
6
	$minAnswerId = 0;
7
	$maxAnswerId = 9;
8
	$wrongAnswerId = 7;
9
	if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
10
		$notAWinner = $aGame->wrongAnswer();
11
	} else {
12
		$notAWinner = $aGame->wasCorrectlyAnswered();
13
	}
14
15
} while ($notAWinner);

Beachten Sie jedoch, dass sich der gesamte Code in einer do-while-Schleife befindet. Müssen wir die Antwort-ID-Variablen jedes Mal neu zuweisen? Ich denke nicht. Versuchen wir, sie aus der Schleife zu entfernen und festzustellen, ob unsere Tests bestanden wurden.

1
$minAnswerId = 0;
2
$maxAnswerId = 9;
3
$wrongAnswerId = 7;
4
do {
5
	$dice = rand(0, 5) + 1;
6
	$aGame->roll($dice);
7
8
	if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
9
		$notAWinner = $aGame->wrongAnswer();
10
	} else {
11
		$notAWinner = $aGame->wasCorrectlyAnswered();
12
	}
13
14
} while ($notAWinner);

Ja. Die Tests bestehen auch so.

Es ist Zeit, zu Game.php zu wechseln und dort auch nach Magic Constants zu suchen. Wenn Sie Code hervorheben, haben Sie sicherlich Konstanten in einer hellen Farbe hervorgehoben. Meine sind blau und ziemlich leicht zu erkennen.

Es war ziemlich einfach, die magische Konstante 50 in dieser for-Schleife zu finden. Wenn wir uns ansehen, was der Code tut, können wir feststellen, dass innerhalb der for-Schleife Elemente in mehrere Arrays verschoben werden. Wir haben also eine Art Liste mit jeweils 50 Elementen. Jede Liste stellt eine Fragenkategorie dar und die Variablen sind tatsächlich Klassenfelder, die oben als Arrays definiert wurden.

1
$this->popQuestions = array();
2
$this->scienceQuestions = array();
3
$this->sportsQuestions = array();
4
$this->rockQuestions = array();

Also, was können 50 darstellen? Ich wette, Sie haben bereits einige Ideen. Das Benennen ist eine der schwierigsten Aufgaben beim Programmieren. Wenn Sie mehr als eine Idee haben und sich nicht sicher sind, welche Sie wählen sollen, schämen Sie sich nicht. Ich habe auch verschiedene Namen im Kopf und prüfe Möglichkeiten, den besten auszuwählen, auch während ich diesen Absatz schreibe. Ich denke, wir können mit einem konservativen Namen für 50 gehen. Etwas in der Art von $questionInEachCategory oder $categorySize oder ähnlichem.

1
$categorySize = 50;
2
for ($i = 0; $i < $categorySize; $i++) {
3
	array_push($this->popQuestions, "Pop Question " . $i);
4
	array_push($this->scienceQuestions, ("Science Question " . $i));
5
	array_push($this->sportsQuestions, ("Sports Question " . $i));
6
	array_push($this->rockQuestions, "Rock Question " . $i);
7
}

Das sieht anständig aus. Wir können es behalten. Und die Tests sind natürlich vorbei.

1
function isPlayable() {
2
	return ($this->howManyPlayers() >= 2);
3
}

Was ist zwei? Ich bin mir sicher, dass Ihnen an diesem Punkt die Antwort klar ist. Das ist einfach:

1
function isPlayable() {
2
	$minimumNumberOfPlayers = 2;
3
	return ($this->howManyPlayers() >= $minimumNumberOfPlayers);
4
}

Sind Sie einverstanden? Wenn Sie eine bessere Idee haben, können Sie diese gerne unten kommentieren. Und deine Tests? Gehen sie noch vorbei?

In der roll()-Methode haben wir nun auch einige Zahlen: zwei, null, 11 und 12.

1
if ($roll % 2 != 0)

Das ist ziemlich klar. Wir werden diesen Ausdruck in eine Methode extrahieren, aber nicht in diesem Tutorial. Wir sind immer noch in der Phase des Verstehens und der Suche nach magischen Konstanten und Saiten. Was ist also mit 11 und 12? Sie sind in der dritten Ebene der if-Anweisungen vergraben. Es ist ziemlich schwer zu verstehen, wofür sie stehen. Vielleicht, wenn wir uns die Linien um sie herum ansehen.

1
if ($this->places[$this->currentPlayer] > 11) {
2
	$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
3
}

Wenn der Platz oder die Position des aktuellen Spielers größer als 11 ist, wird seine Position auf die aktuelle Position minus 12 reduziert. Dies klingt wie ein Fall, in dem ein Spieler das Ende des Bretts oder des Spielfelds erreicht und es in seiner Ausgangsposition neu positioniert wird. Wahrscheinlich Position Null. Wenn unser Spielplan kreisförmig ist, wird der Spieler durch Überschreiten der zuletzt markierten Position auf die relative erste Position gebracht. 11 könnte also die Boardgröße sein.

1
$boardSize = 11;
2
if ($this->inPenaltyBox[$this->currentPlayer]) {
3
	if ($roll % 2 != 0) {
4
		// ... //

5
		if ($this->places[$this->currentPlayer] > $boardSize) {
6
			$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
7
		}
8
		// ... //

9
	} else {
10
		// ... //

11
	}
12
} else {
13
	// ... //

14
	if ($this->places[$this->currentPlayer] > $boardSize) {
15
		$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
16
	}
17
// ... //

18
}

Vergessen Sie nicht, 11 an beiden Stellen innerhalb der Methode zu ersetzen. Das zwingt uns, die Variablenzuweisung außerhalb der if-Anweisungen direkt auf der ersten Einrückungsstufe zu verschieben.

Aber wenn 11 die Größe des Boards ist, was ist 12? Wir subtrahieren 12 von der aktuellen Position des Spielers, nicht 11. Und warum setzen wir die Position nicht einfach auf Null, anstatt zu subtrahieren? Denn das würde unsere Tests zum Scheitern bringen. Unsere vorherige Vermutung, dass der Spieler nach dem Ausführen des Codes in der if-Anweisung auf Position Null landen wird, war falsch. Angenommen, ein Spieler befindet sich auf Position zehn und würfelt eine Vier. 14 ist größer als 11, daher erfolgt die Subtraktion. Der Spieler landet auf Position 10+4-12=2.

Das führt uns zu einer anderen möglichen Benennung für 11 und 12. Ich denke, es ist angemessener, 12 $boardSize aufzurufen. Aber was lässt uns das für 11? Vielleicht $lastPositionOnTheBoard? Ein bisschen lang, aber zumindest sagt es uns die Wahrheit über die magische Konstante.

1
$lastPositionOnTheBoard = 11;
2
$boardSize = 12;
3
if ($this->inPenaltyBox[$this->currentPlayer]) {
4
	if ($roll % 2 != 0) {
5
		// ... //

6
		if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard) {
7
			$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize;
8
		}
9
		// ... //

10
	} else {
11
		// ... //

12
	}
13
} else {
14
	// ... //

15
	if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard) {
16
		$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize;
17
	}
18
// ... //

19
}

Ich weiß, ich weiß! Dort gibt es einige Codeduplikationen. Es ist ziemlich offensichtlich, besonders wenn der Rest des Codes verborgen ist. Aber bitte denken Sie daran, dass wir nach magischen Konstanten suchten. Es wird auch eine Zeit für doppelten Code geben, aber momentan nicht.

Abschließende Gedanken

Ich habe eine letzte magische Konstante im Code belassen. Können Sie es erkennen? Wenn Sie sich den endgültigen Code ansehen, wird er ersetzt, aber das wäre natürlich Betrug. Viel Glück beim Finden und danke fürs Lesen.