Skip to content

Commit

Permalink
Refactor: Avoid Hash[] pattern
Browse files Browse the repository at this point in the history
Hash[] pattern creates many unnecessary intermediate array.
  • Loading branch information
chopraanmol1 committed Jan 11, 2019
1 parent 8091ff6 commit d0a5958
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 40 deletions.
27 changes: 15 additions & 12 deletions lib/roo/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,12 @@ def each(options = {})
clean_sheet_if_need(options)
search_or_set_header(options)
headers = @headers ||
Hash[(first_column..last_column).map do |col|
[cell(@header_line, col), col]
end]
(first_column..last_column).each_with_object({}) do |col, hash|
hash[cell(@header_line, col)] = col
end

@header_line.upto(last_row) do |line|
yield(Hash[headers.map { |k, v| [k, cell(line, v)] }])
yield(headers.each_with_object({}) { |(k, v), hash| hash[k] = cell(line, v) })
end
end
end
Expand Down Expand Up @@ -424,9 +424,9 @@ def find_by_row(row_index)

def find_by_conditions(options)
rows = first_row.upto(last_row)
header_for = Hash[1.upto(last_column).map do |col|
[col, cell(@header_line, col)]
end]
header_for = 1.upto(last_column).each_with_object({}) do |col, hash|
hash[col] = cell(@header_line, col)
end

# are all conditions met?
conditions = options[:conditions]
Expand All @@ -441,9 +441,9 @@ def find_by_conditions(options)
rows.map { |i| row(i) }
else
rows.map do |i|
Hash[1.upto(row(i).size).map do |j|
[header_for.fetch(j), cell(i, j)]
end]
1.upto(row(i).size).each_with_object({}) do |j, hash|
hash[header_for.fetch(j)] = cell(i, j)
end
end
end
end
Expand Down Expand Up @@ -497,8 +497,11 @@ def sanitize_value(v)
def set_headers(hash = {})
# try to find header row with all values or give an error
# then create new hash by indexing strings and keeping integers for header array
@headers = row_with(hash.values, true)
@headers = Hash[hash.keys.zip(@headers.map { |x| header_index(x) })]
header_row = row_with(hash.values, true)
@headers = {}
hash.each_with_index do |(key, _), index|
@headers[key] = header_index(header_row[index])
end
end

def header_index(query)
Expand Down
8 changes: 4 additions & 4 deletions lib/roo/excelx.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ def initialize(filename_or_stream, options = {})
end
end.compact
@sheets = []
@sheets_by_name = Hash[@sheet_names.map.with_index do |sheet_name, n|
@sheets[n] = Sheet.new(sheet_name, @shared, n, sheet_options)
[sheet_name, @sheets[n]]
end]
@sheets_by_name = {}
@sheet_names.each_with_index do |sheet_name, n|
@sheets_by_name[sheet_name] = @sheets[n] = Sheet.new(sheet_name, @shared, n, sheet_options)
end

if cell_max
cell_count = ::Roo::Utils.num_cells_in_range(sheet_for(options.delete(:sheet)).dimensions)
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/excelx/comments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ def comments
def extract_comments
return {} unless doc_exists?

Hash[doc.xpath('//comments/commentList/comment').map do |comment|
doc.xpath('//comments/commentList/comment').each_with_object({}) do |comment, hash|
value = (comment.at_xpath('./text/r/t') || comment.at_xpath('./text/t')).text
[::Roo::Utils.ref_to_key(comment.attributes['ref'].to_s), value]
end]
hash[::Roo::Utils.ref_to_key(comment.attributes['ref'].to_s)] = value
end
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/excelx/images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ def list
def extract_images_names
return {} unless doc_exists?

Hash[doc.xpath('/Relationships/Relationship').map do |rel|
[rel['Id'], "roo" + rel['Target'].gsub(/\.\.\/|\//, '_')]
end]
doc.xpath('/Relationships/Relationship').each_with_object({}) do |rel, hash|
hash[rel['Id']] = "roo" + rel['Target'].gsub(/\.\.\/|\//, '_')
end
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/excelx/relationships.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ def to_a
def extract_relationships
return [] unless doc_exists?

Hash[doc.xpath('/Relationships/Relationship').map do |rel|
[rel.attribute('Id').text, rel]
end]
doc.xpath('/Relationships/Relationship').each_with_object({}) do |rel, hash|
hash[rel.attribute('Id').text] = rel
end
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/roo/excelx/shared_strings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ def create_html(text, formatting)
tmp_str << "<#{elem}>" if val
end
tmp_str << text
reverse_format = Hash[formatting.to_a.reverse]
reverse_format.each do |elem, val|

formatting.reverse_each do |elem, val|
tmp_str << "</#{elem}>" if val
end
tmp_str
Expand Down
8 changes: 4 additions & 4 deletions lib/roo/excelx/sheet_doc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,13 @@ def create_cell_from_value(value_type, cell, formula, format, style, hyperlink,
def extract_hyperlinks(relationships)
return {} unless (hyperlinks = doc.xpath('/worksheet/hyperlinks/hyperlink'))

Hash[hyperlinks.map do |hyperlink|
if hyperlink['id'] && (relationship = relationships[hyperlink['id']])
hyperlinks.each_with_object({}) do |hyperlink, hash|
if relationship = relationships[hyperlink['id']]
target_link = relationship['Target']
target_link += "##{hyperlink['location']}" if hyperlink['location']
[::Roo::Utils.ref_to_key(hyperlink['ref']), target_link]
hash[::Roo::Utils.ref_to_key(hyperlink.attributes["ref"].to_s)] = target_link
end
end.compact]
end
end

def expand_merged_ranges(cells)
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/excelx/styles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ def extract_num_fmt_ids
end

def extract_num_fmts
Hash[doc.xpath('//numFmt').map do |num_fmt|
[num_fmt['numFmtId'], num_fmt['formatCode']]
end]
doc.xpath('//numFmt').each_with_object({}) do |num_fmt, hash|
hash[num_fmt['numFmtId']] = num_fmt['formatCode']
end
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/excelx/workbook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ def sheets

# aka labels
def defined_names
Hash[doc.xpath('//definedName').map do |defined_name|
doc.xpath('//definedName').each_with_object({}) do |defined_name, hash|
# "Sheet1!$C$5"
sheet, coordinates = defined_name.text.split('!$', 2)
col, row = coordinates.split('$')
name = defined_name['name']
[name, Label.new(name, sheet, row, col)]
end]
hash[name] = Label.new(name, sheet, row, col)
end
end

def base_timestamp
Expand Down
6 changes: 3 additions & 3 deletions lib/roo/open_office.rb
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,16 @@ def read_comments(sheet = nil)
end

def read_labels
@label ||= Hash[doc.xpath('//table:named-range').map do |ne|
@label ||= doc.xpath('//table:named-range').each_with_object({}) do |ne, hash|
#-
# $Sheet1.$C$5
#+
name = attribute(ne, 'name').to_s
sheetname, coords = attribute(ne, 'cell-range-address').to_s.split('.$')
col, row = coords.split('$')
sheetname = sheetname[1..-1] if sheetname[0, 1] == '$'
[name, [sheetname, row, col]]
end]
hash[name] = [sheetname, row, col]
end
end

def read_styles(style_elements)
Expand Down

0 comments on commit d0a5958

Please sign in to comment.