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

uniq! returns nil when it is after a shifting operation. #1434

Closed
juneng603 opened this issue Jan 23, 2014 · 4 comments
Closed

uniq! returns nil when it is after a shifting operation. #1434

juneng603 opened this issue Jan 23, 2014 · 4 comments
Labels
Milestone

Comments

@juneng603
Copy link

@juneng603 juneng603 commented Jan 23, 2014

Hi, all.

There is a problem to use uniq! for an array with shifting operations.
It seems to be a bug.

options = [1, 1, 1]
options.shift
p options.uniq
p options.uniq! { |h| h }

The expected result is

[1]
[1]

But, the returned values are

[1]
nil

It should return "[1]" for the second.

The testing environments are
ruby :

$ ruby --version
ruby 2.0.0p353 (2013-11-22 revision 43784) [x86_64-darwin12.5.0]

jruby :

$ ruby --version
jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.6.0_65-b14-462-11M4609 [darwin-x86_64]

and the referenced document is from
http://www.ruby-doc.org/core-2.1.0/Array.html#method-i-uniq

Returns nil if no changes are made (that is, no duplicates are found).
@juneng603
Copy link
Author

@juneng603 juneng603 commented Jan 23, 2014

Here it is a test-case for the issue.

diff --git a/test/test_array.rb b/test/test_array.rb
index e1d1918..903d14d 100644
--- a/test/test_array.rb
+++ b/test/test_array.rb
@@ -19,6 +19,13 @@ class TestArray < Test::Unit::TestCase
     }
   end

+  def test_shift_then_uniq
+    options = [1, 1, 1]
+    options.shift
+    assert_equal [1], options.uniq
+    assert_not_nil options.uniq! { |h| h }
+  end
+
   # JRUBY-4157
   def test_shared_ary_slice
     assert_equal [4,5,6], [1,2,3,4,5,6].slice(1,5).slice!(2,3)
@juneng603
Copy link
Author

@juneng603 juneng603 commented Jan 23, 2014

hi, hannes.

There is a sentence in the manual you mentioned.

Returns nil if no changes are made (that is, no duplicates are found).
therefore, [1, 1, 1] should return [1], not nil.

2014/1/23 Hannes Nevalainen notifications@github.com

This is the expected behavior, read the manual
http://www.ruby-doc.org/core-2.1.0/Array.html#method-i-uniq-21


Reply to this email directly or view it on GitHubhttps://github.com//issues/1434#issuecomment-33118694
.

@bruceadams
Copy link
Contributor

@bruceadams bruceadams commented Jan 23, 2014

Yet another surprise here. The uniq! here does not actually change the contents of options (which, I suppose, explains the nil return--it really didn't make any changes).

options = [1, 1, 1]
options.shift
p options.uniq
p options.uniq! { |h| h }
p options

outputs (using JRuby 1.7.10)

[1]
nil
[1, 1]

Running this on MRI ruby-2.0.0-p353 gives the output is as expected:

[1]
[1]
[1]
@enebo
Copy link
Member

@enebo enebo commented Feb 6, 2014

Re-open if there is more to this but the original reported problem is fixed and it was a blaring mistake in makeHash since we would index from begin but then call elt(i) which already factors begin in its implementation. I can only guess most people using this function got lucky where begin == 0 (which is probably the common case -- begin might not be == 0 because we use Copy on Write semantics for arrays). @grddev thanks for the fix.

@enebo enebo added this to the JRuby 1.7.11 milestone Feb 6, 2014
@enebo enebo added the core label Feb 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants