Skip to content

Conversation

@zverok
Copy link
Contributor

@zverok zverok commented Mar 25, 2018

Reason: Row tries to quack more-or-less like Hash, and Hash has #each_pair. A practical example where it matters:

csv = CSV.parse('Bob,100', headers: %i[name salary])
OpenStruct.new(csv.first)
# NoMethodError: undefined method `each_pair' for #<CSV::Row name:"Bob" salary:"100">

# After this patch:
OpenStruct.new(csv.first)
#=> #<OpenStruct name="Bob", salary="100">

@kou
Copy link
Member

kou commented Mar 31, 2018

Do you want to try adding a test?

@zverok
Copy link
Contributor Author

zverok commented Apr 1, 2018

Thought about it, but I am not sure what is an appropriate amount of testing. It is just an alias, so do you expect me to copy-paste of tests of #each, or just literally test "the alias, named this, exists"?

@kou
Copy link
Member

kou commented Apr 1, 2018

This change isn't so simple.

What is the expected behavior of the following case?

require "csv"
csv = CSV.parse('Bob,100', headers: %i[name salary])
p csv.first.each_pair.to_a
# /tmp/a.rb:3:in `<main>': undefined method `each_pair' for ["Bob", "100"]:Array (NoMethodError)

@zverok
Copy link
Contributor Author

zverok commented Apr 1, 2018

What is the expected behavior of the following case?

On my branch:

csv.first.each_pair.to_a
# => [[:name, "Bob"], [:salary, "100"]]

Is there some problem?..

@kou
Copy link
Member

kou commented Apr 1, 2018

Ah, sorry. I pasted wrong code:

require "csv"
csv = CSV.parse('Bob,100')
p csv.first.each_pair.to_a 

@kou
Copy link
Member

kou commented Apr 1, 2018

Ah. it's not related with this change.
It's my fault.

@kou kou merged commit 3e77167 into ruby:master Apr 2, 2018
@zverok zverok deleted the row-each-pair branch May 18, 2018 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants