はじめに
この記事は、僕が最近同期(めちゃ仕事できるマン)と仕事をしていてコードレビューで指摘をされて「なるほど！」と感動したので
その感動を忘れないようにするための備忘録みたいなものです。
レガシーな例
今回は例として、奇数と偶数を分ける処理を書いてみます。
やり方としては…
- 奇数と偶数を分けた結果を同じ配列の異なるキーに格納する
呼び出し側はこんな感じです。
DivideTask.php
<?php
require_once "./divider.php";
class DivideTask
{
public function execDivide() {
$number = array(1, 2, 3, 4, 5);
$divider = $this->getDivider();
$divided_array = $divider->divide($number);
if (!empty($divided_array["even"])) {
echo "-------偶数--------\n";
foreach ($divided_array["even"] as $even) {
echo $even . "\n";
}
}
if (!empty($divided_array["odd"])) {
echo "-------奇数--------\n";
foreach ($divided_array["odd"] as $odd) {
echo $odd . "\n";
}
}
}
private function getDivider() {
return new Divider();
}
}
$divideTask = new DivideTask();
$divideTask->execDivide();
実際に偶数と奇数に分ける処理はこんな感じです。
Divider.php
<?php
class Divider
{
/**
* 受け取った数字を偶数と奇数に分ける処理
* @param array $numbers 偶数と奇数の数字が混合している配列
* @return array $result 振り分けが完了した配列
* ex) ["even"=>2, "odd"=>3]
*/
public function divide(array $numbers) {
$result = array();
foreach($numbers as $number) {
if($number % 2 === 0) {
$result["even"][] = $number;
} else {
$result["odd"][] = $number;
}
}
return $result;
}
}
-------偶数--------
2
4
-------奇数--------
1
3
5
動作としてはこれで全然問題は無いのですが、以下の点において問題があります。
-
divide()メソッドを呼び出す側は、返ってくる配列の中にevenとoddの２つのキーがある前提で呼んでいる - 「素数も追加してよ」と言われた時は新しく配列にキーを追加する必要があり、呼び出し元も呼び出し側も変えなければならない
- 「奇数いらなくなったし消していいよ」と言われた瞬間に
oddキーが一瞬でレガシーになる
まあともかく、今のままだと仕様の変更に脆くレガシーになりやすいということです。
そこで今回はデータクラスを活用してみたいと思います。
リファクタリング
データクラスを作成します。
偶数と奇数をそれぞれクラスプロパティとして定義をし、メソッドはセッターとゲッターだけです。
Number.php
<?php
class Number
{
public evenNumber = array();
public oddNumber = array();
public function getEvenNumber() {
return $this->evenNumber;
}
public function getOddNumber() {
return $this->oddNumber;
}
public function setEvenNumber($number) {
$this->evenNumber[] = $number;
}
public function setOddNumber($number) {
$this->oddNumber[] = $number;
}
}
DividerクラスでこのNumberクラスのインスタンスを作成し、偶数と奇数をそれぞれプロパティに追加をします。
追加が終わった段階で、オブジェクトをそのままDivideTaskクラスにreturnをしてあげます。
Dividerクラスをリファクタします。
主な変更点としては以下の通りです。
- 結果を配列に追加するのではなく、オブジェクトに持たせる
- 返却する結果もそのままオブジェクトで返す
Divider.php
<?php
require_once "./Number.php";
class Divider
{
/**
* 受け取った数字を偶数と奇数に分ける処理
* @param array $numbers 偶数と奇数の数字が混合している配列
* @return Object Number
*/
public function divideOddOrEven(array $numbers) {
/* ①結果を配列ではなくオブジェクトに持たせる */
//$result = array();
$numberObject = $this->getNumberObject();
foreach($numbers as $number) {
if($number % 2 === 0) {
/* ②数字をSetterを使ってプロパティに格納していく */
//$result["even"][] = $number;
$numberObject->setEvenNumber($number);
} else {
/* ②数字をSetterを使ってプロパティに格納していく */
//$result["odd"][] = $number;
$numberObject->setOddNumber($number);
}
}
/* ③返却する結果もオブジェクトをそのまま返す */
//return $result;
return $numberObject;
}
/**
* @return Object Number
*/
private function getNumberObject() {
return new Number();
}
}
処理を呼ぶ側もリファクタしましょう。
DivideTask.php
<?php
require_once "./divider.php";
class DivideTask
{
public function execDivide() {
$number = array(1, 2, 3, 4, 5);
$divider = $this->getDivider();
/* ①返ってくる結果はObjectなので、それをメソッド内で活用する */
//$divided_array = $divider->divideOddOrEven($number);
$numberObject = $divider->divideOddOrEven($number);
/* ②Getterメソッドを使って現在のNumberクラスのプロパティを取得する */
$evenNumber = $numberObject->getEvenNumber();
$oddNumber = $numberObject->getOddNumber();
if (!empty($evenNumber)) {
echo "-------偶数--------\n";
foreach ($evenNumber as $even) {
echo $even . "\n";
}
}
if (!empty($oddNumber)) {
echo "-------奇数--------\n";
foreach ($oddNumber as $odd) {
echo $odd . "\n";
}
}
}
private function getDivider() {
return new Divider();
}
}
$divideTask = new DivideTask();
$divideTask->execDivide();
おわりに
イマサラタウンな話題ですが、ふとした時に忘れがちで油断するとリファクタ前みたいなコード(なんでも配列のキーに追加マン)になってしまうので、改めてこのタイミングでアウトプットをすることで初めて自分のものに出来た感があります。
最後まで読んでくださり、ありがとうございました(｀･ω･´)ゞﾋﾞｼｯ!!