Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/csv/row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,13 @@ def ==(other)
# row.to_h # => {"Name"=>"Foo"}
def to_h
hash = {}
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
each do |key, value|
new_key, new_value = if block_given?
yield(key, value)
else
[key, value]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
end

or

each do |key, value|
  key, value = yield(key, value) if block_given?
  # ...
end

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a good one

end
Comment on lines +677 to +679
hash[new_key] = new_value unless hash.key?(new_key)
end
hash
end
Expand Down
3 changes: 3 additions & 0 deletions test/csv/test_row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,9 @@ def test_to_hash
assert_predicate(string_key, :frozen?)
assert_same(string_key, @row.headers[h])
end
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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create a separated test?

end

def test_to_csv
Expand Down
Loading