Ruby の静的解析ツール一式をまとめて実行できる RubyCritic を使って静的解析をしていたところ、
Reek の Boolean Parameter の警告がでてきて怒られました。
Boolean Parameter Smell
Reekのドキュメントにおける Boolean Parameter の臭いの説明は以下です。
Boolean Parameter is a case of Control Couple, where a method parameter is defaulted to true or false.
A Boolean Parameter effectively permits a method's caller to decide which execution path to take.
This is a case of bad cohesion. You're creating a dependency between methods that is not really necessary, thus increasing coupling.
Boolean Parameter - docs - troessner/reek
問題点
フラグがあるということは、そのメソッドは2つの役割をするということです。
その役割の結合度は増し、一つの変更が及ぼす影響が大きくなります。
また、その引数が名前付き引数ではない場合、呼び出し元のコードを見ただけでは
何の動作を切り替えているのか把握できません。
class Numbers
def self.list(limit, include_zero)
if include_zero
[*0..limit]
else
[*1..limit]
end
end
end
# true, false で何が変わるかコードを見てもわからない
print Numbers.list(10, true), "\n" # [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
print Numbers.list(10, false), "\n" # [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
改善
メソッドを分けることで依存を減らし、呼び元からも分かりやすくします。
class Numbers
def self.list_include_zero(limit)
[*0..limit]
end
def self.list_exclude_zero(limit)
[*1..limit]
end
end
print Numbers.list_include_zero(10), "\n" # [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
print Numbers.list_exclude_zero(10), "\n" # [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]