当たり前じゃん!と言われるかもしれないけど、ハマったのでメモ。
ハマった事象
Ruby で以下のようなコードを書いていた。
(あくまでサンプルなので細かいとこにツッコミたくなってもグッとこらえてください)
def all_negative?(items)
results = items.map do |item|
return false unless item.is_a?(Numeric)
item < 0 ? true : false
end
results.all?
end
引数に配列を与えて、すべての要素が負の数であるかどうかを判定するメソッドであるが、数値以外は数値と比較できないので、Integer でなければ予め return false
する、という処理を書いてある。
このメソッドを実行すると一見ちゃんと動く。
all_negative?([-1, -2])
# => true
all_negative?([-1, :hoge, 1])
# => false
しかしこれ、実は return false
をした時点で、ループをぶった切って all_negative?
自体が false
を返してしまっている。
map は戻り値で配列を作るんだ!と思い込んでたけど、 return
しちゃうとまずいということを知らなかった。
試しに results.all?
でなく results?
を返すようにしてやるとよくわかる。
def all_negative?(items)
results = items.map do |item|
return false unless item.is_a?(Numeric)
item < 0 ? true : false
end
results
end
all_negative?([-1, -2])
# => [true, true]
all_negative?([-1, :hoge, 1])
# => false
# => 本当は [true, false, false] となっていて欲しい
後者は配列ではなく直接 false を返してしまっている。
今回の場合ではたまたま偶然困らないけど、例えばループ内で何か別の処理をするような場合、メソッドの戻り値は正しいけど処理されてないものがある…ということになりかねない。というかなった。
本来はどう書くべきか
今回のような場合であれば、return
ではなく next
を使うべきだった。
next
は引数を与えてやればその引数を返すようになるので、return
をそのまま next
に変えてやればOK。
def all_negative?(items)
results = items.map do |item|
next false unless item.is_a?(Numeric)
item < 0 ? true : false
end
results
end
all_negative?([-1, -2])
# => [true, true]
all_negative?([-1, :hoge, 1])
# => [true, false, false]