Project

General

Profile

Actions

Bug #16908

open

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

Added by ioquatix (Samuel Williams) 12 months ago. Updated about 2 months 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) 12 months 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) 12 months 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) 12 months 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) 12 months 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) 12 months 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) 12 months ago

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

Updated by matz (Yukihiro Matsumoto) about 2 months ago

I don't remember why I made this behavior. Now I think #shift should return nil for an empty hash, without calling its default value, in the long run. naruse (Yui NARUSE) claims the change should be postponed to 3.2 or later if we make the change.

Matz.

Actions

Also available in: Atom PDF