縦に切るか、横に切るか
関数は小さく作るべきだが、ただ単に小さく作れば良いという話ではない
例えば、テーブルを扱うようなアプリケーションを書く時に、
行のComponentを作ってloopさせるか
列のComponentを作ってloopさせるか
は、全く異なる意味になる
この切り方で実装者の意図が乗ることになるし、切り方がミスってるとその後の実装が大変になる
良い悪い例を見つけた
とあるイケてないフレームワークに以下のようなmethodがあった
編集画面のController内のedit()という名前のmethod
以下の2つのことを行っているクソデカmethod
requestにproduct_idが含まれているならば更新とみなして、既存のデータを更新する
requestにnewが含まれている場合は、新規作成とみなす
こんな感じの実装になっている
おそらく、処理の後半が共通しているから一緒にしたれ、と実装されたんだろう
あるいは、返り値のtwig templateが同じものだからか(これも良くないと思う)
code:php
function edit($id) {
$has_class = false;
$ProductStock = null;
if(is_null($id) {
// 新規作成の場合のProductとかの定義
}else {
// 更新用にProductのデータとかを用意
}
// クソ長いが、このへんは大まかには同じ処理
return [
// nullだったりはするが返す項目自体は同じ
]
}
こういうmethodをリファクタする際に2つのアプローチが考えられる
①条件分岐部分をmethodに切り出す
そのmethod内(ここではgetProduct)で同じ条件分岐のようなものをやる
code:php
function edit($id) {
$Product = $this->getProduct($id); // この中で条件分岐
// クソ長いが、このへんは大まかには同じ処理
return [
// nullだったりはするが返す項目自体は同じ
]
}
②入口自体を分ける
code:php
function new() {
// 新規作成の場合のProductとかの定義
// クソ長いが、このへんは大まかには同じ処理
return [
// nullだったりはするが返す項目自体は同じ
// templateも別物を用意して、必要な分だけ返すようにしたい
]
}
function edit($id) {
// 更新用にProductのデータとかを用意
// クソ長いが、このへんは大まかには同じ処理
return [
// nullだったりはするが返す項目自体は同じ
// templateも別物を用意して、必要な分だけ返すようにしたい
]
}
後半部分の処理は同じなので重複がめっちゃある感じに見える
断然②のほうが良いと思うmrsekut.icon
①は「たまたま同じ処理ぽかった」から共通して書いているだけ
責務に着目していない
新規作成と編集はそもそも責務が異なるので別物として定義すべき クソ長いが、このへんは大まかには同じ処理の部分の処理も
個々の関数の中身が5行程度なら重複しているとみなさないだろう
code:php
function new() {
$Product = getProduct();
$Hoge = getHoge();
$Piyo = getPiyo();
}
function edit() {
$Product = getProduct(); // たまたまnew()と同じだっただけ
$Hoge = getHoge(); // たまたまnew()と同じだっただけ
$Fuga = getFuga();
}
リファクタリングの方針としては、
1 まずnewとeditに分ける
2 その後、newとeditを個別に関数を小さく切り出していく
その中でたまたま共通の関数を使える部分があるならば、同じ関数を使用する
この1と2の順序を間違うと、これまた同じ直行性の問題で関数の切り出し方を間違う恐れがあるmrsekut.icon
他の例
ListLがあって、これの要素ごとに複数の処理をしたいときのことを考える
以下の2つの方策がある
List単位でやっていく
複数の処理単位でやっていく
準備
code:ts
// 行いたい複数の処理。(配列にではなく)要素に対して行う
const f = <T>(e: T) => ...;
const g = <T>(e: T) => ...;
const h = <T>(e: T) => ...;
// f,g,hのリスト版
const fs = <T>(arr: T[]) => arr.map((e) => f(e));
const gs = <T>(arr: T[]) => arr.map((e) => g(e));
const hs = <T>(arr: T[]) => arr.map((e) => h(e));
前者 List単位でやっていく
リストの全要素に対してfを行った後に、全要素に対してgを行う、..を繰り返す
code:ts
const main = () => {
const a = fs(list);
const b = gs(a);
return hs(b);
};
後者 複数の処理単位でやっていく
リストの1要素に対して、f,g,hを全部行った後に、次の要素に対して同じ処理を行う...を繰り返す
code:ts
const main2 = () => {
return list.map((e) => {
const a = f(e);
const b = g(a);
return h(b);
});
};
同期処理か非同期処理かで考え方は微妙に変わりそう
非同期処理の場合IOが多分に重なったりするので、前者のほうが良かったりするかもしれない
fsの実装は変えたほうが良い、とかにはなりそうだけど
ここでの前提条件ならどっちでやっても大きな変わりは無い気がする
関数の切り方に殆ど違いがないという意味