Project

General

Profile

Bug #16908

Strange behaviour of Hash#shift when used with `default_proc`.

Added by ioquatix (Samuel Williams) about 1 month ago. Updated about 1 month ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:98486]

Description

I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]

My feeling was, both cases should return [nil, 0].

Updated by jeremyevans0 (Jeremy Evans) about 1 month ago

While your particular example is non-intuitive, there is a simple explanation for it. The first time hash.shift is called, hash is empty, so it returns the default value (0). It gets the default value by calling the default_proc for hash with a nil key. There is no better option since hash.shift isn't provided a key. The second time hash.shift is called, the hash is not empty, so it returns the first entry as a key value pair. I agree Hash#shift semantics with a default_proc are questionable, but I'm not sure if it could be improved.

I don't think we should change this behavior. It is expected that Hash.new.shift should return nil, as should Hash.new(nil).shift and Hash.new{}.shift.

hash.shift is used in conditionals:

hash = {a: 1, b: 2}
while (k,v = hash.shift)
  p [k, v]
end

If you change Hash#shift to return an array when the hash is empty, you've turned this into an infinite loop.

Updated by ioquatix (Samuel Williams) about 1 month ago

jeremyevans0 (Jeremy Evans) I agree with your assessment, however that hash does not have default_proc so I assume that it would return nil after all key-value pairs are shifted out. That's different from invoking default_proc and returning [nil, 0].

Updated by jeremyevans0 (Jeremy Evans) about 1 month ago

ioquatix (Samuel Williams) wrote in #note-2:

jeremyevans0 (Jeremy Evans) I agree with your assessment, however that hash does not have default_proc so I assume that it would return nil after all key-value pairs are shifted out. That's different from invoking default_proc and returning [nil, 0].

Same principle applies when using a default_proc:

# simple hash with indifferent access
hash = Hash.new{|h,k| h[k.to_s] unless k.is_a? String}
hash['a'] = 1
hash['b'] = 2
while (k,v = hash.shift)
  p [k, v]
end

Updated by Eregon (Benoit Daloze) about 1 month ago

Maybe Hash#shift should not call the default_proc or use Hash#default?
I.e., it would always return nil if Hash#empty?.
I think that would be more intuitive and probably compatible enough.

Updated by Eregon (Benoit Daloze) about 1 month ago

In other words, the current semantics of return hash.default(nil) if hash.empty? feel hacky and actually harmful to me.
The user probably never expects to have the default_proc called with a nil key in many cases.

Updated by Dan0042 (Daniel DeLorme) about 1 month ago

I would expect Hash#shift to return either a key-value tuple or nil. Returning the default value is, honestly, incomprehensible.

Also available in: Atom PDF