Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add specs for Array#reject!, delete_if 2.3 change #273

Merged
merged 1 commit into from
Jul 12, 2016

Conversation

jrafanie
Copy link
Contributor

From #175:
Array#select!, Array#keep_if, Array#reject!, and Array#delete_if
no longer changes the receiver array instantly every time the
block is called. Feature #10714

Note, select! and keep_if's behavior seems to be unchanged with 2.3.
I tested with 2.3.1, 2.2.5, 2.1.8, and 2.0.0-p648 and they don't mutate the
receiver after each block, only after all blocks.


ruby_version_is "2.3" do
it "updates the receiver after all blocks" do
@object.send(@method) do |i|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nitpick: could you update the block variable to be e?
i sounds like index but here it's an element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, 👍

@eregon
Copy link
Member

eregon commented Jul 12, 2016

Thanks a lot, looks good!

From ruby#175:
Array#select!, Array#keep_if, Array#reject!, and Array#delete_if
no longer changes the receiver array instantly every time the
block is called. Feature #10714

Note, select! and keep_if's behavior seems to be unchanged with 2.3.
I tested with 2.3.1, 2.2.5, 2.1.8, and 2.0.0-p648 and they don't mutate the
receiver after each block, only after all blocks.
@jrafanie jrafanie force-pushed the add_2_3_tests_for_reject_bang branch from f0ed330 to e68a19a Compare July 12, 2016 21:21
@jrafanie
Copy link
Contributor Author

@eregon Updated. I didn't want to change unrelated block variables to e also since others use i so I kept it only to my changes. I agree, it makes more sense.

@eregon
Copy link
Member

eregon commented Jul 12, 2016

The changes while iterating in keep_if/select! can also be observed, but it's trickier as the size is only changed at the very end. The elements however might be reassigned while iterating:

[1] pry(main)> a = [1,2,3,4,5]
=> [1, 2, 3, 4, 5]
[2] pry(main)> a.keep_if { |e| p a; e.even? }
[1, 2, 3, 4, 5]
[1, 2, 3, 4, 5]
[2, 2, 3, 4, 5]
[2, 2, 3, 4, 5]
[2, 4, 3, 4, 5]
=> [2, 4]
[3] pry(main)> RUBY_VERSION
=> "2.2.3"

You are right though it seems there is not much difference between latest 2.2 and 2.3.
Maybe there was a change in between which never got in a release.

@eregon eregon merged commit 8e65d52 into ruby:master Jul 12, 2016
@eregon
Copy link
Member

eregon commented Jul 12, 2016

Thank you for your contribution, it's much appreciated!

If you feel on a roll, you can try spec'ing keep_if/select! in more details according to my previous comment 😃

I didn't want to change unrelated block variables to e

Yes absolutely, I meant only those in your changes.
Then it's my job to maintain the rest :)

@jrafanie jrafanie deleted the add_2_3_tests_for_reject_bang branch July 12, 2016 21:54
@eregon eregon mentioned this pull request Jul 13, 2016
51 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants