Support block in CSV::Row#to_h#356
Conversation
| new_key, new_value = if block_given? | ||
| yield(key, value) | ||
| else | ||
| [key, value] |
There was a problem hiding this comment.
We don't want to create a needless temporary Array.
Could you use
if block_given?
each do |key, value|
key, value = yield(key, value)
# ...
end
else
each do |key, value|
# ...
end
endor
each do |key, value|
key, value = yield(key, value) if block_given?
# ...
end?
There was a problem hiding this comment.
Thanks, that's a good one
| row2 = CSV::Row.new(%w{A B C}, [1, 2, 3]) | ||
| hash2 = row2.to_hash { |k, v| [k, v ** 2] } | ||
| assert_equal({"A" => 1, "B" => 4, "C" => 9}, hash2) |
| each do |key, value| | ||
| key, value = yield(key, value) if block_given? |
There was a problem hiding this comment.
Ah, sorry. We need to call self[key]:
| each do |key, value| | |
| key, value = yield(key, value) if block_given? | |
| each do |key, _value| | |
| value = self[key] | |
| key, value = yield(key, value) if block_given? |
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
There was a problem hiding this comment.
Pull request overview
Adds support for passing a block to CSV::Row#to_h / #to_hash, aligning usage with Ruby’s to_h { ... } pattern and avoiding the need for row.entries.to_h { ... }.
Changes:
- Add a new test covering
CSV::Row#to_hashwith a block. - Update
CSV::Row#to_hto yield each key/value pair to an optional block before insertion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/csv/test_row.rb | Adds a test for block form of to_hash. |
| lib/csv/row.rb | Implements yielding to an optional block in to_h during hash construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? | ||
|
|
||
| hash[key] = value unless hash.key?(key) |
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? |
| def to_h | ||
| hash = {} | ||
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? | ||
|
|
||
| hash[key] = value unless hash.key?(key) |
| row = CSV::Row.new(%w{A B C}, [1, 2, 3]) | ||
| hash = row.to_hash { |k, v| [k, v ** 2] } |
| def to_h | ||
| hash = {} | ||
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? |
| hash = {} | ||
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| next if hash.key?(key) |
There was a problem hiding this comment.
key may be changed by yield(key, value). So we can't do this if block_given?.
How about using separated each?
if block_given?
each do |key, _value|
key, value = yield(key, self[key])
hash[key] = value unless hash.key?(key)
end
else
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
end
endThere was a problem hiding this comment.
Pull request overview
Adds block support to CSV::Row#to_h (and therefore #to_hash) to align with Ruby’s to_h { ... } conversion pattern, so callers don’t need to go through row.entries.to_h { ... }.
Changes:
- Extend
CSV::Row#to_hto accept an optional block that transforms each (key, value) pair before insertion. - Document the new block form in
CSV::Row#to_h’s RDoc. - Add a unit test exercising
to_hashwith a block.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/csv/test_row.rb | Adds coverage for CSV::Row#to_hash accepting a block. |
| lib/csv/row.rb | Implements and documents block support for CSV::Row#to_h/#to_hash. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| each do |key, _value| | ||
| hash[key] = self[key] unless hash.key?(key) | ||
| next if hash.key?(key) | ||
|
|
||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? | ||
|
|
||
| hash[key] = value |
| next if hash.key?(key) | ||
|
|
||
| value = self[key] | ||
| key, value = yield(key, value) if block_given? |
| row = CSV::Row.new(%w{A A B C}, [1, 2, 2, 3]) | ||
| hash = row.to_hash { |k, v| [k, v**2] } | ||
| assert_equal({"A" => 1, "B" => 4, "C" => 9}, hash) |
It will be consistent with Ruby's method, otherwise it has to be like
row.entries.to_h { ... }