if文は関数(or メソッド)内に書くべきなのでしょうか? それとも、関数(or メソッド)呼び出し前にif文を書くべきなのでしょうか?
リファクタリングの一つのテクニックとして、if、for、while文はメソッド化してしまうというものがあるようです。私が見た複数のサンプルコードは全て、以下に掲載したパターンAのような書き方でした。つまり、関数(or メソッド)内にif文を含めてしまう書き方です。
個人的には以下に掲載したパターンBのように、if文は関数(or メソッド)実行前に書いた方がコードを読み返すときにわかりやすいと感じたのですが、パターンAのような書き方をするのには何か理由があるのでしょうか?
※以下コード内の「__」はスペースの代わり
★リファクタリング前
function doSomething () {
__var condition = true;
__if (!condition) {
____return;
__}
__//ここにif条件がtrueの場合に実行するコード
}
★リファクタリング後 - パターンA
function doSomething () {
__var condition = true;
__doSomethingElse();
}
function doSomethingElse() {
__if (!condition) {
____return;
__}
__//ここにif条件がtrueの場合に実行するコード
}
★リファクタリング後 - パターンB
function doSomething () {
__var condition = true;
__if (!condition) {
____return;
__}
__doSomethingElse();
}
function doSomethingElse() {
__//ここにif条件がtrueの場合に実行するコード
}
みんなの回答 6 件
リファクタリングするならまず複雑なコードの部分からじゃない?
条件分岐をどこでするかは、好きにすればいい。
本質的じゃないと思います。つまりどちらでもよい。Aは個人的に読みづらいと感じます。
関数があっちもこっちも発生するとスパゲッティコードになるよ。
何度も(3回以上)出現しないかぎり、1つの関数内でまとめるべき
A は解りづらい。どっちにしても doSomethingElse() をあちこちから呼び出すのでなければ、メソッド化せずに親関数内に展開した方がいい。つまり元のコードで十分(笑)。
誰もホントの答え言わないのなw
わかってないだけなのかもしれないが。
1機能1メソッドで、メソッドを細かく定義するのは、オブジェクト指向の考え方としていいと思う。
(外部から呼び出されるか否かに関わらず、1メソッド内で複数の機能の処理はしない。)
同時にクラス設計もちゃんと考えないと、めちゃくちゃになるけど。
.
リファクタ前のtrueの時の処理が、1つの機能の処理しかしてないなら、
個人的にはリファクタ前のようなコードを書く。
.
普段コーディングする場合には、パターンBで書いてる。
パターンAのような、親のdoSomething()で判定したフラグ変数を、
子のdoSomethingElse()が参照してるのに違和感がある。
.
でも、doSomethingElse( getCondition() ) とかで、
conditionが引数として渡される場合は、
パターンBのように、doSomethingElse側でガードしてreturnするかも。
関連するトピックス