DiffDaily

Deep & Concise - OSS変更の定点観測

[Rails] Array#in_groupsに入力値検証を追加

rails/rails

なぜこの変更が必要だったのか

RailsのActiveSupportには、配列を複数のグループに分割する便利なメソッドとして Array#in_groupsArray#in_groups_of が用意されています。しかし、Array#in_groups には引数の妥当性チェックが実装されていませんでした。

#56608 で報告されたように、in_groups(0)in_groups(-1) のような無効な引数を渡すと、意味不明なエラーメッセージが表示されていました。一方、類似メソッドの in_groups_of には適切な入力検証が実装されており、この不整合が問題視されていました。

実装された変更内容

入力値検証ロジックの追加

Array#in_groups メソッドに、以下の検証ロジックが追加されました。

def in_groups(number, fill_with = nil, &block)
  if number.to_i <= 0
    raise ArgumentError,
      "Number of groups must be a positive integer, was #{number.inspect}"
  end

  number = number.to_i
  # 既存の実装が続く...
end

この変更により、以下の挙動が保証されます。

  1. 正の整数のみ受け付ける: number.to_i <= 0 の場合は ArgumentError を発生させる
  2. 明確なエラーメッセージ: 何が問題だったのかを具体的に示す
  3. 整数への変換: 検証後に number.to_i で整数化することで、浮動小数点数も適切に処理できる

テストケースの拡充

今回の変更では、以下のテストケースが追加されました。

def test_in_groups_invalid_argument
  assert_raises(ArgumentError) { [].in_groups(0) }
  assert_raises(ArgumentError) { [].in_groups(-1) }
  assert_raises(ArgumentError) { [].in_groups(nil) }
end

def test_in_groups_with_float
  result = [1, 2, 3, 4, 5].in_groups(2.9)
  assert_equal 2, result.size
  assert_equal [[1, 2, 3], [4, 5, nil]], result
end

注目すべき点として、浮動小数点数 2.9 を渡した場合、to_i により 2 として処理されます。これは既存の in_groups_of メソッドと同じ挙動です。

実用例

変更前(エラーメッセージが不明瞭):

[1, 2, 3].in_groups(0)
# => 不明瞭なエラーまたは予期しない挙動

変更後(明確なエラーメッセージ):

[1, 2, 3].in_groups(0)
# => ArgumentError: Number of groups must be a positive integer, was 0

[1, 2, 3].in_groups(-1)
# => ArgumentError: Number of groups must be a positive integer, was -1

[1, 2, 3].in_groups(nil)
# => ArgumentError: Number of groups must be a positive integer, was nil

正常な使用例:

[1, 2, 3, 4, 5].in_groups(2)
# => [[1, 2, 3], [4, 5, nil]]

[1, 2, 3, 4, 5].in_groups(2.9)  # 浮動小数点数も可能
# => [[1, 2, 3], [4, 5, nil]]  # 2グループとして処理される

まとめ

この変更により、Array#in_groupsArray#in_groups_of の挙動が統一され、不正な引数に対して一貫性のあるエラーハンドリングが実現されました。開発者はより明確なフィードバックを得られるようになり、デバッグ時間の短縮が期待できます。