Bug #8739

Range#last と#last(n)で挙動の統一がされていない

Added by Tomoya Ueda about 2 years ago. Updated about 2 years ago.

[ruby-dev:47587]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:ruby 2.1.0dev (2013-08-05) [x86_64-linux] Backport:1.9.3: UNKNOWN, 2.0.0: UNKNOWN

Description

Range#lastは終端を返しますが、#last(n)は最後の要素を返します。
引数の有無で挙動が異なるのは意図したものでしょうか?
意図したものであれば、返す値を終端か要素どちらかに挙動を統一出来ないでしょうか。

#終端が返る
(1...5).last => 5
#最後の要素が返る
(1...5).last(1) => [4]

Associated revisions

Revision 42400
Added by Nobuyoshi Nakada about 2 years ago

range.c: consider exclusive

  • range.c (range_last): exclude the last number of the exclusive range if the end is Numeric. [Bug #8739]

Revision 42400
Added by Nobuyoshi Nakada about 2 years ago

range.c: consider exclusive

  • range.c (range_last): exclude the last number of the exclusive range if the end is Numeric. [Bug #8739]

Revision 42403
Added by Nobuyoshi Nakada about 2 years ago

net/http/header.rb: remove a hack

  • lib/net/http/header.rb (Net::HTTPHeader#set_range): remove a hack for the bug fixed at [Bug #8739].

Revision 42403
Added by Nobuyoshi Nakada about 2 years ago

net/http/header.rb: remove a hack

  • lib/net/http/header.rb (Net::HTTPHeader#set_range): remove a hack for the bug fixed at [Bug #8739].

Revision 42404
Added by Nobuyoshi Nakada about 2 years ago

json/add/range.rb: use Range#end

  • ext/json/lib/json/add/range.rb (Range#as_json): use Range#end instead of Range#last which can be affected by Range#exclusive?. ref [Bug #8739]

Revision 42404
Added by Nobuyoshi Nakada about 2 years ago

json/add/range.rb: use Range#end

  • ext/json/lib/json/add/range.rb (Range#as_json): use Range#end instead of Range#last which can be affected by Range#exclusive?. ref [Bug #8739]

Revision 42407
Added by Nobuyoshi Nakada about 2 years ago

range.c: return nil for empty range

  • range.c (range_last): return nil for empty range, or in the case the predecessor is smaller than the begin. [Bug #8739]

Revision 42407
Added by Nobuyoshi Nakada about 2 years ago

range.c: return nil for empty range

  • range.c (range_last): return nil for empty range, or in the case the predecessor is smaller than the begin. [Bug #8739]

Revision 42431
Added by Nobuyoshi Nakada about 2 years ago

range.c: revert r42400

Revision 42431
Added by Nobuyoshi Nakada about 2 years ago

range.c: revert r42400

Revision 42432
Added by Yui NARUSE about 2 years ago

Follow r42431

Follow tests to revert r42400. [Bug #8739]

Revision 42432
Added by Yui NARUSE about 2 years ago

Follow r42431

Follow tests to revert r42400. [Bug #8739]

Revision 42434
Added by Nobuyoshi Nakada about 2 years ago

test_range.rb: remove invalid assertions

  • test/ruby/test_range.rb: remove invalid assertions for [Bug #8739].

Revision 42434
Added by Nobuyoshi Nakada about 2 years ago

test_range.rb: remove invalid assertions

  • test/ruby/test_range.rb: remove invalid assertions for [Bug #8739].

History

#1 Updated by Yukihiro Matsumoto about 2 years ago

  • Status changed from Open to Rejected
  • Assignee set to Yukihiro Matsumoto

意図したものです。

引数がない時には「終端」が欲しいでしょうし、引数を指定して複数値を取り出す時に、eachで登場しない値を含めるものおかしいと思いました。

仮に変えるとして「どちらかに揃える」ことを希望されるならば、どちらに揃えるべきだとも思いますか?
その時に「揃える」ことが現状の動作よりどう嬉しいのか、非互換性を導入するだけの価値があるのか教えてください。

Matz.

#2 Updated by Yukihiro Matsumoto about 2 years ago

  • Status changed from Rejected to Closed

むむ、リビジョン 42400で最後の要素を返す(結果として挙動を揃える非互換な方向の)変更が行われてますね。

これを機会に改めて考えると、終端は「end」メソッドを使うようにして、「last」の挙動は他のEnumerableと揃えることにしましょう。
非互換ですが、なんとかなる、でしょう、多分。

標準添付ライブラリにあった非互換問題は、リビジョン 42403、42404、42407で修正されています。

ちょっと押し切られた感じですが、まあ、長い目で見ればこれで良かったんでしょう、きっと。

Matz.

#3 Updated by Yui NARUSE about 2 years ago

  • Target version set to 2.1.0
  • Assignee changed from Yukihiro Matsumoto to Nobuyoshi Nakada
  • Status changed from Closed to Assigned

仕様を変えるときは NEWS に書いて下さい。
あと、rdoc もちゃんと追従させて下さい。

#4 Updated by Yui NARUSE about 2 years ago

なんかrubyspecのこけ方がおかしいと思ったのでよく見てみたら、rubyspec自体(=mspec)が
Range#last を ruby_version_is "1.9"..."1.9.3" とかの判定で使っているようで、
見事に踏んだようです。
http://c64b.rubyci.org/~chkbuild/ruby-trunk/log/20130806T090301Z.diff.html.gz

最低でも移行措置必要じゃないですかねぇ。

#5 Updated by Motohiro KOSAKI about 2 years ago

だれかが、ギャっと言ったら君主豹変スで態度を変えてrevertすべきなんじゃないですかねえ。

#6 Updated by Yui NARUSE about 2 years ago

  • Priority changed from Normal to 5

kosaki (Motohiro KOSAKI) wrote:

だれかが、ギャっと言ったら君主豹変スで態度を変えてrevertすべきなんじゃないですかねえ。

だいたい「ぎゃっ」というのはリリース後な上に、だいたいblogやtwitterでの愚痴として露出することになるところ、
標準ライブラリでの2件に加えてrubyspec (mspec)も踏んでるあたり、これはもう予告無しでの変更はアウトだろうと思うんですよ。
柴田さん側での見解も聞きますかね。

#7 Updated by Nobuyoshi Nakada about 2 years ago

Rejectされているの気づかず、うっかり変更してしまいましたが、revertしますかねぇ。
ただし、net/http/header.rbのように、last(1)[0]と同じモノが欲しいケースはあるのではないかと思います。

rubyspectの件は、("1.9"..."1.9.3").to_aを使うのはまずいってことですねぇ。

やっぱりrevertかな。

#8 Updated by Tomoya Ueda about 2 years ago

コメントいただきありがとうございます。

リビジョン 42400の変更のようになると、直感的でうれしいとは思いましたが、
「非互換性を導入するだけの価値」をあまり念頭に置いていない上での発言でした。

揃えてほしいという意見は変わりませんが、
非互換の影響を目にし、それ以上のメリットを私には説明することもできません。

ご判断お願い致します。

#9 Updated by Nobuyoshi Nakada about 2 years ago

  • % Done changed from 0 to 100
  • Status changed from Assigned to Closed

This issue was solved with changeset r42431.
Tomoya, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


range.c: revert r42400

#10 Updated by Hiroshi SHIBATA about 2 years ago

呼ばれたので返事をすると、2.1.0 で warning 出して 2.1.1 で変えるか、2.0.x で warning 出して 2.1 で変えるというところが妥当と思いますが、後者はもう期限切れな気がします。

Also available in: Atom PDF