Feature #20953: Array#fetch_values vs #values_at protocols

• Status: Open
• Assignee: matz (Yukihiro Matsumoto)
• Target version: -

[zverok (Victor Shepelev)]

I believe that the user might expect #fetch_values to be a stricter version of #values_at, confirming to the same protocol for arguments.

But the current implementation for #fetch_values is simpler:

[1, 2, 3, 4, 5].values_at(0, 3..4)    #=> [1, 4, 5]
[1, 2, 3, 4, 5].fetch_values(0, 3..4) # TypeError: in 'Array#fetch': no implicit conversion of Range into Integer

I believe aligning the implementations would lessen confusion (even if it makes #fetch_values implementation somewhat less trivial).

The practical example of usefulness:

HEADERS = %w[Name Department]

def table_headers(rows)
  HEADERS.fetch_values(...rows.map(&:size).max) { '<unknown'> }
  # Or, alternatively:
  # HEADERS.fetch_values(...rows.map(&:size).max) { raise ArgumentError, "No header defined for column #{it + 1}" }
end

table_headers([
  ['John'],
  ['Jane'],
]) #=> ["Name"]

table_headers([
  ['John'],
  ['Jane', 'Engineering'],
]) #=> ["Name", "Department"]

table_headers([
  ['John', 'Accounting', 'Feb 24'],
  ['Jane', 'Engineering'],
]) #=> ["Name", "Department", "<unknown>"]
# or ArgumentError No header defined for column 3

(Obviously, we can use fetch_values(*(0...max_row_size)) as an argument, but it feels like an unjustified extra work when values_at already can do this.)

[byroot (Jean Boussier)]

Array#fetch_values is modeled after Hash#fetch_values, not Array#values_at.

Since Array "keys" can only possibly be integers, it makes sense for a method that is specific to Array to cast the arguments this way.

But for the method that is meant to be used indiscriminately with either Array or Hash, I don't think it's a good idea.

[zverok (Victor Shepelev)]

Array#fetch_values is modeled after Hash#fetch_values, not Array#values_at.

But in Hash, #values_at and #fetch_values have exactly the same protocol (they accept the list of keys), and can be substituted by each other.

So, I understand it is all hand-wavy, yet I’d say that the common motive of similar methods between Hash and Array is that they are, well, similar, yet internal consistency inside one class is more important than consistency between classes.

In fact, we don’t need to go far to prove that: values_at is a good example: the same name, the similar signature values_at(*objects), and yet Array’s one accepts ranges besides just indices. There are many more methods with the same name and sense (#[], #select/#reject, #include?, etc.), all of them behaving slightly differently between classes, and matching the expectation of a particular class users.

But for the method that is meant to be used indiscriminately with either Array or Hash, I don't think it's a good idea.

Do we have a lot of those, and what’s the reasoning/use cases behind this? I mean, methods that are exactly reconciled between Array and Hash (at the expense of the internal consistency inside any one class), to be used indiscriminately?

[byroot (Jean Boussier)]

Do we have a lot of those, and what’s the reasoning/use cases behind this?

Things like #dig, so that you can traverse a tree of mixed Hash/Array with some sort of "path" argument.

Either way, I see pros and cons to both behavior, so it's not for me to decide, it's up to @matz (Yukihiro Matsumoto).

The issue though is the release is imminent and the next developer meeting is scheduled for January. So if you wish to change this before the release you'll have to ping @matz (Yukihiro Matsumoto) on Twitter or something for him to make a decision in time.

[zverok (Victor Shepelev)]

• Assignee set to matz (Yukihiro Matsumoto)

[mame (Yusuke Endoh)]

• Related to Feature #20702: Add `Array#fetch_values` added

[mame (Yusuke Endoh)]

Discussed at the dev meeting. @matz (Yukihiro Matsumoto) liked to align the behavior of Array#fetch_values with Array#values_at here in this case. So a Range should be expanded.

[byroot (Jean Boussier)]

Alright.

What's the expected behavior of:

[1, 2, 3].fetch_values(42..)
[1, 2, 3].fetch_values(42..) { true }
[1, 2, 3].fetch_values(..42)
[1, 2, 3].fetch_values(..42) { true }

[byroot (Jean Boussier)]

I opened https://github.com/ruby/ruby/pull/12565 as a draft, with some failing specs for the edge cases I found but I don't know how they should be handled.

[zverok (Victor Shepelev)]

What's the expected behavior of:

For ..42 versions, I believe that values_at behavior gives enough affordance:

[1, 2, 3].values_at(..6)
#=> [1, 2, 3, nil, nil, nil, nil]

So, I suppose:

[1, 2, 3].fetch_values(..6)
# IndexError at 3, same as...
[1, 2, 3].fetch_values(0, 1, 2, 3, 4, 5, 6)

[1, 2, 3].fetch_values(..6) { true }
#=> [1, 2, 3, true, true, true, true]

For 42.. it is not that straightforward, because values_at behavior is somewhat confusing here.

[1, 2, 3].values_at(6..)
#=> []

We might just follow it, though. I believe that it is implicitly treated as

[1, 2, 3].values_at(*(6..2).to_a)

(i.e. the apper bound is (ary.size - 1)), and, as this range is empty, nothing is tried to be fetched from the array. So it should just be the same:

[1, 2, 3].fetch_values(6..)
#=> [] 

It is not an immediately obvious thing, but I am not sure what would be the one; and at least this behavior is already established.

Alternatively, if the range beginning is out of bounds, fetch_values might raise. This would be explainable, too.