Refactoring Legacy Code: Bagian 2 - Magic String & konstanta

Indonesian (Bahasa Indonesia) translation by Sandi Muamar (you can also view the original English article)

Kode lama. Kode jelek. Kode yang rumit. Kode spaghetti. Jibberish omong kosong. Dalam 2 kata, Legacy code. Ini adalah seri yang akan membantu Anda bekerja dan berurusan dengan itu.

Kami pertama kali bertemu legacy code kami dalam pelajaran kita sebelumnya. Kemudian kami berlari kode untuk membentuk pendapat tentang fungsi dasar dan menciptakan tes Golden Master suite memiliki jaring pengaman mentah untuk perubahan nantinya. Kami akan terus bekerja pada kode ini dan Anda dapat menemukannya di bawah folder trivia/php_start. folder lain trivia/php_start adalah dengan pelajaran ini kodekode yang selesai.

Waktu untuk perubahan pertama datang dan apa yang lebih baik untuk memahami code base sulit daripada mulai untuk mengekstrak magic constant dan string ke dalam variabel? Tugas ini tampaknya sederhana akan memberi kita wawasan yang lebih besar dan kadang-kadang tak terduga ke dalam legacy code. Kita perlu mengetahui niat dari penulis asli kode dan menemukan nama untuk potongan-potongan kode yang belum pernah kita lihat sebelumnya.

Magic String

Sihir string yang digunakan langsung dalam berbagai ekspresi, tanpa diberikan kepada variabel string. String semacam ini memiliki arti khusus untuk penulis asli kode, tapi bukan menugaskan mereka untuk variable yang jelas, penulis berpikir makna dari string adalah cukup jelas.

Mengidentifikasi String pertama untuk perubahan

Mari kita mulai dengan melihat Game.php dan mencoba untuk mengidentifikasi string. Jika Anda menggunakan IDE (dan Anda harus) atau text editor yang lebih cerdar yang mampu menyoroti kode sumber, spotting string akan mudah. Berikut ini adalah gambaran bagaimana kode terlihat seperti pada tampilan.

Semuanya dengan warna orage adalah string. Menemukan setiap string dalam kode sumber kami sangat mudah dengan cara ini. Jadi, pastikan highligting supported dan diaktifkan di editor Anda, apa pun adalah aplikasi Anda.

Bagian orange pertama dalam kode kita yang langsung pada baris tiga. Namun string mengandung hanya karakter newline. Ini harus cukup jelas menurut pendapat saya, sehingga kami dapat melanjutkan.

Ketika datang untuk memutuskan apa yang harus di ekstrak dan apa yang tidak perlu dirubah, ada beberapa thumbs-up, tetapi pada akhirnya itu adalah pendapat profesional yang harus menang. Berdasarkan itu, Anda akan memiliki untuk memutuskan apa yang harus dilakukan dengan setiap potongan kode Anda analis.

for ($i = 0; $i < 50; $i++) {
  	array_push($this->popQuestions, "Pop Question " . $i);
		array_push($this->scienceQuestions, ("Science Question " . $i));
		array_push($this->sportsQuestions, ("Sports Question " . $i));
		array_push($this->rockQuestions, $this->createRockQuestion($i));
	}
}

function createRockQuestion($index) {
	return "Rock Question " . $index;
}

Jadi mari kita menganalisis baris 32 hingga 42, potongan yang dapat Anda lihat di atas. Untuk pop, ilmu pengetahuan, dan olahraga pertanyaan, ada hanya sebuah gabungan sederhana. Namun, action untuk menulis string untuk pertanyaan rock diekstraksi ke dalam sebuah metode. Menurut pendapat Anda, concatenations dan string ini cukup jelas sehingga kami dapat menjaga semua dari mereka di dalam for loop? Atau, Apakah Anda pikir mengekstrak semua string ke metode mereka akan membenarkan keberadaan metode tersebut? Jika demikian, bagaimana Anda menamai metode tersebut?

Memperbarui Golder Master dan tes

Terlepas dari jawabannya, kita akan perlu untuk memodifikasi kode. Sekarang saatnya untuk menempatkan Golder Master untuk bekerja dan menulis ujian kami yang benar-benar berjalan dan membandingkan kode kita dengan konten yang ada.

class GoldenMasterTest extends PHPUnit_Framework_TestCase {

	private $gmPath;

	function setUp() {
		$this->gmPath = __DIR__ . '/gm.txt';
	}

	function testGenerateOutput() {
		$times = 20000;
		$this->generateMany($times, $this->gmPath);
	}

	function testOutputMatchesGoldenMaster() {
		$times = 20000;
		$actualPath = '/tmp/actual.txt';
		$this->generateMany($times, $actualPath);
		$file_content_gm = file_get_contents($this->gmPath);
		$file_content_actual = file_get_contents($actualPath);
		$this->assertTrue($file_content_gm == $file_content_actual);
	}

	private function generateMany($times, $fileName) {...}

	private function generateOutput($seed) {...}
}

Kami membuat satu lagi test untuk membandingkan output, memastikan testGenerateOutput() hanya menghasilkan output sekali dan bukan yang lain. Kami juga memindahkan golder master output file "gm.txt" ke direktori saat ini karena "/tmp" dapat dibersihkan ketika sistem reboot. Untuk hasil aktual kita, kita masih bisa menggunakan itu. Pada kebanyakan UNIX seperti sistem "/tmp" Mount ke dalam RAM sehingga jauh lebih cepat daripada sistem file. Jika Anda melakukannya dengan baik, tes harus sukses tanpa masalah.

Hal ini sangat penting untuk mengingat untuk menandai tes generator kami untuk "dilewatkan" untuk perubahan nanti. Jika Anda merasa lebih nyaman dengan komentar atau bahkan menghapus sema, silahkan melakukannya. Penting bahwa Golden Master tidak akan berubah ketika kita mengubah kode kita. Itu dihasilkan sekali dan kita tidak ingin untuk mengubahnya, pernah, sehingga kita dapat yakin kode baru yang dihasilkan kita selalu membandingkan dengan yang asli. Jika Anda merasa lebih nyaman melakukan cadangan itu, silakan lanjutkan untuk melakukannya.

function testGenerateOutput() {
	$this->markTestSkipped();
	$times = 20000;
	$this->generateMany($times, $this->gmPath);
}

Aku hanya menandai tes sebagai dilewati. Ini akan menempatkan hasil tes kami menjadi "yellow", yang berarti semua tes lewat tetapi beberapa melewatkan atau ditandai sebagai tidak lengkap.

Membuat perubahan pertama kami

Dengan tes di tempat, kita dapat mulai mengubah kode. Dalam pendapat profesional, Semua string dapat disimpan di dalam for loop. Kami akan mengambil kode dari createRockQuestion() metode, bergerak di dalam for loop, dan menghapus metode semaunya. Refactoring ini disebut metode Inline.

"Menempatkan bodi metode ke dalam body yang pemanggil dan menghapus metode." ~ Martin Fowler

Ada seperangkat tertentu langkah-langkah untuk melakukan jenis refactoring, sebagaimana didefinisikan oleh Marting Fowler di Refactoring: meningkatkan desain kode yang ada:

Periksa bahwa metode ini tidak polimorfik.
Menemukan semua pemanggilan metode.
Ganti setiap pemanggilan dengan metode body.
Kompilasi dan test.
Hapus definisi metode.

Kami tidak punya subclass extend Game, jadi langkah pertama memvalidasi.

hanya ada satu penggunaan metode kami, di dalam for loop.

function  __construct() {

	$this->players = array();
	$this->places = array(0);
	$this->purses = array(0);
	$this->inPenaltyBox = array(0);

	$this->popQuestions = array();
	$this->scienceQuestions = array();
	$this->sportsQuestions = array();
	$this->rockQuestions = array();

	for ($i = 0; $i < 50; $i++) {
		array_push($this->popQuestions, "Pop Question " . $i);
		array_push($this->scienceQuestions, ("Science Question " . $i));
		array_push($this->sportsQuestions, ("Sports Question " . $i));
		array_push($this->rockQuestions, "Rock Question " . $i);
	}
}

function createRockQuestion($index) {
	return "Rock Question " . $index;
}

Kami menempatkan kode dari createRockQuestion() ke dalam for loop di konstruktor. Kami masih memiliki kode lama kami. Sekarang saatnya untuk menjalankan pengujian kami.

Pengujian kami sukses. Kita dapat menghapus metode createRockQuestion() kami.

function  __construct() {

	// ... //

	for ($i = 0; $i < 50; $i++) {
		array_push($this->popQuestions, "Pop Question " . $i);
		array_push($this->scienceQuestions, ("Science Question " . $i));
		array_push($this->sportsQuestions, ("Sports Question " . $i));
		array_push($this->rockQuestions, "Rock Question " . $i);
	}
}

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

Akhirnya kita harus menjalankan test kami lagi. Jika kita memanggil ke metode, mereka akan gagal.

Mereka harus sukses lagi. Congrats! Kami selesai dengan refactoring pertama kami.

String lain untuk pertimbangkan

String metode add() dan roll() hanya digunakan untuk output mereka menggunakan metode echoln(). askQuestions() membandingkan string untuk kategori. Hal ini tampaknya dapat diterima juga. currentCategory() di sisi lain mengembalikan string berdasarkan nomor. Dalam metode ini, ada banyak string digandakan. Mengubah kategori apapun, kecuali Rock akan memerlukan mengubah namanya di tiga tempat, hanya dalam metode ini.

function currentCategory() {
	if ($this->places[$this->currentPlayer] == 0) {
		return "Pop";
	}
	if ($this->places[$this->currentPlayer] == 4) {
		return "Pop";
	}
	if ($this->places[$this->currentPlayer] == 8) {
		return "Pop";
	}
	if ($this->places[$this->currentPlayer] == 1) {
		return "Science";
	}
	if ($this->places[$this->currentPlayer] == 5) {
		return "Science";
	}
	if ($this->places[$this->currentPlayer] == 9) {
		return "Science";
	}
	if ($this->places[$this->currentPlayer] == 2) {
		return "Sports";
	}
	if ($this->places[$this->currentPlayer] == 6) {
		return "Sports";
	}
	if ($this->places[$this->currentPlayer] == 10) {
		return "Sports";
	}
	return "Rock";
}

Saya pikir kita bisa melakukan lebih baik. Kita dapat menggunakan metode refactoring disebut Introduce Local Variable dan menghilangkan duplikasi. Ikuti petunjuk ini:

Menambahkan variabel dengan nilai yang diinginkan.
Menemukan semua penggunaan nilai.
Ganti semua penggunaan dengan variabel.

function currentCategory() {
	$popCategory = "Pop";
	$scienceCategory = "Science";
	$sportCategory = "Sports";
	$rockCategory = "Rock";

	if ($this->places[$this->currentPlayer] == 0) {
		return $popCategory;
	}
	if ($this->places[$this->currentPlayer] == 4) {
		return $popCategory;
	}
	if ($this->places[$this->currentPlayer] == 8) {
		return $popCategory;
	}
	if ($this->places[$this->currentPlayer] == 1) {
		return $scienceCategory;
	}
	if ($this->places[$this->currentPlayer] == 5) {
		return $scienceCategory;
	}
	if ($this->places[$this->currentPlayer] == 9) {
		return $scienceCategory;
	}
	if ($this->places[$this->currentPlayer] == 2) {
		return $sportCategory;
	}
	if ($this->places[$this->currentPlayer] == 6) {
		return $sportCategory;
	}
	if ($this->places[$this->currentPlayer] == 10) {
		return $sportCategory;
	}
	return $rockCategory;
}

Ini jauh lebih baik. Anda mungkin telah mengamati beberapa perbaikan nantinya yang bisa kami lakukan untuk kode kita, tapi kita hanya pada awal pekerjaan kami. Hal ini menggoda untuk memperbaiki semuanya Anda menemukan segera, tapi tolong jangan. Beberapa kali, terutama sebelum kode baik dipahami, menggoda perubahan dapat menyebabkan buntu atau kode lebih rusak. Jika Anda berpikir ada kesempatan Anda akan lupa ide Anda, hanya dicatat pada sticky note atau membuat tugas dalam perangkat lunak manajemen proyek Anda. Sekarang kita perlu untuk melanjutkan dengan string masalah terkait.

Dalam sisa file, Semua string adalah output terkait, dikirim ke echoln(). Untuk saat ini, kita akan meninggalkan mereka tak tersentuh. Memodifikasi mereka akan mempengaruhi pencetakan dan memberikan logika aplikasi kita. Mereka adalah bagian dari lapisan presentasi yang dicampur dengan logika bisnis. Kita akan berurusan dengan memisahkan berbagai kekhawatiran dalam pelajaran nantinya.

Magic Constnant

Magic constant itu sama seperti magic string, tetapi dengan nilai-nilai. Nilai-nilai ini dapat nilai-nilai boolean atau angka. Kita akan berkonsentrasi terutama pada angka-angka yang digunakan dalam if statement atau return statement atau ungkapan lain. Jika angka-angka ini memiliki makna yang jelas, kita perlu ekstrak mereka ke dalam variabel atau metode.

include_once __DIR__ . '/Game.php';

$notAWinner;

$aGame = new Game();

$aGame->add("Chet");
$aGame->add("Pat");
$aGame->add("Sue");

do {

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

	if (rand(0, 9) == 7) {
		$notAWinner = $aGame->wrongAnswer();
	} else {
		$notAWinner = $aGame->wasCorrectlyAnswered();
	}

} while ($notAWinner);

Kita akan mulai dengan GameRunner.php saat ini dan fokus pertama kami adalah metode roll() yang mendapat beberapa nomor acak. Penulis sebelumnya tidak peduli untuk memberikan angka-angka makna. Bisakah kita? Jika kita menganalisis kode:

rand(0, 5) + 1

Itu akan mengembaliakn nomor antara satu dan enam. Bagian acak mengembalikan sejumlah antara nol dan lima yang kami selalu menambahkan satu. Jadi pasti antara satu dan enam. Sekarang kita perlu mempertimbangkan konteks aplikasi kita. Kami sedang mengembangkan permainan trivia. Kita tahu ada beberapa jenis papan di mana pemain kami harus bergerak. Dan untuk melakukannya, kita perlu untuk melempar dadu. Mati memiliki enam wajah dan dapat menghasilkan nomor antara satu dan enam. Yang tampaknya seperti pengurang wajar.

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

Menyenangkan bukan? Kami menggunakan Introduce Local Variable konsep refactoring lagi. Kami menamai variabel baru $dice dan mewakili nomor acak yang dihasilkan antara satu dan enam. Hal ini juga membuat statement kami berikutnya terdengar benar-benar alami: Game, dadu roll.

Apakah Anda menjalankan tes Anda? Saya tidak menyebutkan itu, tapi kita perlu untuk menjalankan mereka sesering mungkin. Jika Anda belum, ini akan menjadi waktu yang baik untuk menjalankan mereka. Dan mereka harus sukses.

Jadi, ini adalah kasus tidak lebih dari sekadar bertukar nomor dengan variabel. Kami mengambil ekspresi seluruh yang mewakili sejumlah dan diekstraksi ke dalam variabel. Ini dapat secara teknis dianggap kasus Magic konstan, tetapi bukan suatu kasus. Apa tentang random expression berikutnya?

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

Hal ini lebih rumit. Apa yang nol, sembilan dan tujuh dalam ekspresi itu? Mungkin kita dapat menamai mereka. Pada pandangan pertama, aku tidak punya baik ide untuk nol dan sembilan, jadi mari kita coba tujuh. Jika nomor yang mengembalikan oleh fungsi random kami setara dengan tujuh, kita akan memasuki cabang pertama if statement yang menghasilkan jawaban yang salah. Jadi mungkin tujuh bisa dinamai $wrongAnswerId.

$wrongAnswerId = 7;
if (rand(0, 9) == $wrongAnswerId) {
	$notAWinner = $aGame->wrongAnswer();
} else {
	$notAWinner = $aGame->wasCorrectlyAnswered();
}

Pengujian kami masih sukses dan kode ini agak lebih ekspresif. Sekarang bahwa kami berhasil untuk nama kami Bilangan tujuh, menempatkan bersyarat ke dalam konteks yang berbeda. Kita dapat memikirkan beberapa nama yang layak untuk nol dan sembilan juga. Mereka adalah hanya parameter untuk rand(), sehingga variabel yang akan mungkin dinamakan min-sesuatu dan max-sesuatu.

$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;
if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
	$notAWinner = $aGame->wrongAnswer();
} else {
	$notAWinner = $aGame->wasCorrectlyAnswered();
}

Sekarang itu ekspresif. Kami memiliki ID jawaban minimum, maksimum satu dan satu lagi untuk jawaban yang salah. Misteri terpecahkan.

do {

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

	$minAnswerId = 0;
	$maxAnswerId = 9;
	$wrongAnswerId = 7;
	if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
		$notAWinner = $aGame->wrongAnswer();
	} else {
		$notAWinner = $aGame->wasCorrectlyAnswered();
	}

} while ($notAWinner);

Tapi perhatikan bahwa semua kode yang ada di dalamnya do-while loop. Kita perlu menetapkan kembali variabel ID jawaban setiap kali? Saya kira tidak. Mari kita coba untuk memindahkan mereka keluar dari loop dan melihat jika tes sukses.

$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;
do {
	$dice = rand(0, 5) + 1;
	$aGame->roll($dice);

	if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId) {
		$notAWinner = $aGame->wrongAnswer();
	} else {
		$notAWinner = $aGame->wasCorrectlyAnswered();
	}

} while ($notAWinner);

Ya. Tes sukses seperti ini juga.

Saatnya untuk beralih ke Game.php dan mencari Magic konstanta disana juga. Jika Anda memiliki kode highlighting, Anda pasti memiliki konstanta yang disorot dalam beberapa warna cerah. biru dan mereka cukup mudah dikenali.

Menemukan 50 konstan magic dalam for loop itu cukup mudah. Dan jika kita melihat apa kode tidak, kita dapat menemukan bahwa dalam for loop, elemen push ke beberapa array. Jadi kita memiliki beberapa jenis daftar, masing-masing dengan 50 elemen. Setiap lagu melambangkan kategori pertanyaan dan variabel yang benar-benar bidang kelas yang didefinisikan di atas sebagai array.

$this->popQuestions = array();
$this->scienceQuestions = array();
$this->sportsQuestions = array();
$this->rockQuestions = array();

Jadi, apa yang dapat 50 mewakili? Saya yakin Anda sudah memiliki beberapa ide. Penamaan adalah salah satu tugas yang paling sulit dalam pemrograman, jika Anda memiliki lebih dari satu ide dan Anda merasa tidak yakin yang satu untuk memilih, jangan malu. Saya juga memiliki berbagai nama di kepala saya dan saya mengevaluasi kemungkinan untuk memilih yang terbaik, bahkan saat menulis paragraph ini. Saya pikir kita bisa pergi dengan nama konservatif untuk 50. Sesuatu di sepanjang baris $questionsInEachCategory atau $categorySize atau sesuatu yang serupa.

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

Yang terlihat layak. Kita dapat tetap. Dan tes tentu lewat.

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

Apa itu dua? Aku yakin pada titik ini jawabannya jelas bagi Anda. Ini mudah:

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

Apakah Anda setuju? Jika Anda memiliki gagasan yang lebih baik, jangan ragu untuk komentar di bawah ini. Dan tes Anda? Mereka masih sukses?

Sekarang, dalam metode roll() kita memiliki beberapa nomor juga: dua, nol, 11 dan 12.

if ($roll % 2 != 0)

Ini cukup jelas. Kami akan ekstrak expression itu ke metode, tapi tidak dalam tutorial ini. Kami masih dalam tahap pemahaman dan berburu untuk magic constant dan string. Jadi apa tentang 11 dan 12? Mereka dikubur dalam tingkat ketiga if statement. Hal ini cukup sulit untuk memahami apa yang mereka perjuangkan. Mungkin jika kita melihat garis-garis di sekitar mereka.

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

Jika posisi atau tempat pemain saat ini adalah lebih besar dari 11, maka posisinya akan dikurangi untuk satu saat ini dikurangi 12. Ini terdengar seperti kasus ketika seorang pemain mencapai akhir bidang papan atau bermain dan itu adalah reposisi dalam posisi awal. Mungkin posisi nol. Atau, jika papan permainannya melingkar, akan lebih dari posisi terakhir ditandai akan menaruh pemain dalam posisi pertama relatif. Jadi 11 bisa ukuran papan.

$boardSize = 11;
if ($this->inPenaltyBox[$this->currentPlayer]) {
	if ($roll % 2 != 0) {
		// ... //
		if ($this->places[$this->currentPlayer] > $boardSize) {
			$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
		}
		// ... //
	} else {
		// ... //
	}
} else {
	// ... //
	if ($this->places[$this->currentPlayer] > $boardSize) {
		$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
	}
// ... //
}

Jangan lupa untuk mengganti 11 di kedua tempat di dalam metode. Hal ini akan memaksa kita untuk memindahkan assignment variabel di luar if statement, tepat di tingkat indentasi yang pertama.

Tapi jika 11 papan ukuran, terus 12? Kami kurangi 12 dari pemain posisi saat ini, bukan 11. Dan mengapa tidak kita hanya menetapkan posisi nol bukan mengurangi? Karena itu akan membuat tes gagal. Dugaan kami sebelumnya bahwa pemain akan berakhir sampai pada posisi nol setelah kode di dalamnya if statement dijalankan, ternyata salah. Mari kita mengatakan seorang pemain berada dalam posisi sepuluh dan empat roll. 14 lebih besar dari 11, sehingga pengurangan akan terjadi. Pemain akan berakhir di posisi 10 + 4-12 = 2.

Ini mendorong kita ke arah lain mungkin penamaan untuk 11 dan 12. Saya pikir itu lebih tepat untuk memanggil 12 $boardSize. Tapi apa bagaimana meninggalkan kita untuk 11? Mungkin $lastPositionOnTheBoard? Sedikit panjang, tapi setidaknya itu memberitahu kita kebenaran tentang magic constant.

$lastPositionOnTheBoard = 11;
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
	if ($roll % 2 != 0) {
		// ... //
		if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard) {
			$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize;
		}
		// ... //
	} else {
		// ... //
	}
} else {
	// ... //
	if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard) {
		$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - $boardSize;
	}
// ... //
}

Aku tahu, aku tahu! Ada beberapa kode duplikasi. Hal ini cukup jelas, terutama dengan sisa kode tersembunyi. Tetapi Harap diingat bahwa kami setelah magic constant. Akan ada waktu untuk duplikasi kode juga, tapi tidak sekarang.

Akhir kata

Aku meninggalkan satu terakhir magic constant dalam kode. Dapatkah Anda melihat itu? Jika Anda melihat kode akhir, itu akan diganti, tapi tentu saja akan ada kecurangan. Good luck menemukan ini dan terima kasih sudah membaca.