Project

General

Profile

Actions

Backport #7947

closed

Queue#clear の返り値が Queue 内部の配列になっている

Added by clicube (cube cube) about 11 years ago. Updated about 11 years ago.


Description

■現象
Queue#clear

def clear
@que.clear
end
と実装されていて, Array#clear は self を返すので,結果的に内部の配列 @que が返っています.

■問題と思われること
1.
内部で使っている変数にアクセスできてしまいます.
(これは instance_eval でアクセスしようと思えばできるわけですが)
2.
Array も Queue も #push や #pop があるため,
Queue#clear の返り値に対してメソッドチェーンで #push などを繋いだりすると,
エラーが起こらないにもかかわらず内部の変数を直接書き換えてしまう可能性があると考えます.

■パッチについて
今回はQueue#clearで気づきましたが,
QueueおよびSizedQueueのメソッドについて Array との対称性を考えると,
・Queue#push
・Queue#clear
・SizedQueue#push
・SizedQueue#clear
は self が返るべきかと考えました.
以上4メソッドについて変更を加えるパッチを添付します.

テストの実行結果は以下です.
$ ./ruby -I./lib -I. test/thread/test_queue.rb
Run options:

Running tests:

Finished tests in 1.680776s, 7.1396 tests/s, 13.6842 assertions/s.
12 tests, 23 assertions, 0 failures, 0 errors, 0 skips

ruby -v: ruby 2.0.0dev (2013-02-24 trunk 39476) [x86_64-darwin11.4.2]

パッチのつくり方やテストの書き方/実行方法に自信がないのですが,これでいいのでしょうか.
IRCで相談に乗っていただいた方々,ありがとうございました.


Files

queue_retval.patch (1.15 KB) queue_retval.patch clicube (cube cube), 02/25/2013 02:31 AM

Updated by kosaki (Motohiro KOSAKI) about 11 years ago

  • Category set to core
  • Status changed from Open to Assigned
  • Assignee set to kosaki (Motohiro KOSAKI)
  • Target version set to 2.1.0

review ok です。あとで簡単なテストして取り込んでおきます

Updated by kosaki (Motohiro KOSAKI) about 11 years ago

committed at r39713 and r39714.

Actions #3

Updated by kosaki (Motohiro KOSAKI) about 11 years ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby master to Backport200
  • Category deleted (core)
  • Assignee changed from kosaki (Motohiro KOSAKI) to nagachika (Tomoyuki Chikanaga)
  • Target version deleted (2.1.0)

Updated by nagachika (Tomoyuki Chikanaga) about 11 years ago

  • Status changed from Assigned to Closed

元の挙動はバグ扱いすべきものだとは思うので悩みましたけど、一応挙動が変化してしまうのでバックポートは見合わせようと思います。主なデメリットはメソッドチェーンができないということで、現在のバグに依存しているコードがある可能性と天秤にかけると 2.0.0 では互換性のほうに重きを置きたいと思います。

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0