LoginSignup
29
15

More than 5 years have passed since last update.

Ruby の map の中で return すると外側のメソッドを return してしまう

Last updated at Posted at 2018-02-21

当たり前じゃん!と言われるかもしれないけど、ハマったのでメモ。

ハマった事象

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]

参考: rubyのnextに引数与えるとnil返さずにすむのしらなんだ

29
15
8

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
29
15