From 00356c2e4534607f838e6a0c72a6feb8fd689280 Mon Sep 17 00:00:00 2001 From: chopraanmol1 Date: Fri, 21 Sep 2018 21:33:40 +0530 Subject: [PATCH] Inherit Roo::Excelx::Coordinate from Array Profiling Script ``` MemoryProfiler.report{ Roo::Excelx.new(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }.pretty_print ``` Master: ``` Total allocated: 34559100 bytes (504994 objects) Total retained: 5563403 bytes (103022 objects) allocated memory by gem ----------------------------------- 19254338 roo/lib 8509100 nokogiri-1.8.4 6793822 rubyzip-1.2.2 1304 tmpdir 320 weakref 216 other allocated objects by gem ----------------------------------- 358381 roo/lib 145846 nokogiri-1.8.4 735 rubyzip-1.2.2 22 tmpdir 8 weakref 2 other retained memory by gem ----------------------------------- 5561094 roo/lib 792 rubyzip-1.2.2 725 nokogiri-1.8.4 320 weakref 296 tmpdir 176 other retained objects by gem ----------------------------------- 102989 roo/lib 14 nokogiri-1.8.4 8 weakref 7 rubyzip-1.2.2 3 tmpdir 1 other ``` After Patch: ``` Total allocated: 33439850 bytes (477013 objects) Total retained: 4444153 bytes (75041 objects) allocated memory by gem ----------------------------------- 18135098 roo/lib 8509100 nokogiri-1.8.4 6793812 rubyzip-1.2.2 1304 tmpdir 320 weakref 216 other allocated objects by gem ----------------------------------- 330400 roo/lib 145846 nokogiri-1.8.4 735 rubyzip-1.2.2 22 tmpdir 8 weakref 2 other retained memory by gem ----------------------------------- 4441854 roo/lib 782 rubyzip-1.2.2 725 nokogiri-1.8.4 320 weakref 296 tmpdir 176 other retained objects by gem ----------------------------------- 75008 roo/lib 14 nokogiri-1.8.4 8 weakref 7 rubyzip-1.2.2 3 tmpdir 1 other ``` --- lib/roo/excelx/coordinate.rb | 15 ++++++---- lib/roo/excelx/sheet_doc.rb | 6 ++-- lib/roo/utils.rb | 18 +++++------- spec/lib/roo/utils_spec.rb | 9 ++++++ test/excelx/test_coordinate.rb | 51 ++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 21 deletions(-) create mode 100644 test/excelx/test_coordinate.rb diff --git a/lib/roo/excelx/coordinate.rb b/lib/roo/excelx/coordinate.rb index acd08aeb..4f61b178 100644 --- a/lib/roo/excelx/coordinate.rb +++ b/lib/roo/excelx/coordinate.rb @@ -1,15 +1,18 @@ module Roo class Excelx - class Coordinate - attr_accessor :row, :column + class Coordinate < ::Array def initialize(row, column) - @row = row - @column = column + super() << row << column + freeze end - def to_a - @array ||= [row, column].freeze + def row + self[0] + end + + def column + self[1] end end end diff --git a/lib/roo/excelx/sheet_doc.rb b/lib/roo/excelx/sheet_doc.rb index bade7ac4..9402ddd3 100755 --- a/lib/roo/excelx/sheet_doc.rb +++ b/lib/roo/excelx/sheet_doc.rb @@ -44,8 +44,7 @@ def each_cell(row_xml) # If you're sure you're not going to need this hyperlinks you can discard it coordinate = ::Roo::Utils.extract_coordinate(cell_element[COMMON_STRINGS[:r]]) hyperlinks = unless @options[:no_hyperlinks] - key = coordinate.to_a - hyperlinks(@relationships)[key] + hyperlinks(@relationships)[coordinate] end yield cell_from_xml(cell_element, hyperlinks, coordinate) @@ -200,8 +199,7 @@ def extract_cells(relationships) extracted_cells = {} doc.xpath('/worksheet/sheetData/row/c').each do |cell_xml| coordinate = ::Roo::Utils.extract_coordinate(cell_xml[COMMON_STRINGS[:r]]) - key = coordinate.to_a - extracted_cells[key] = cell_from_xml(cell_xml, hyperlinks(relationships)[key], coordinate) + extracted_cells[coordinate] = cell_from_xml(cell_xml, hyperlinks(relationships)[coordinate], coordinate) end expand_merged_ranges(extracted_cells) if @options[:expand_merged_ranges] diff --git a/lib/roo/utils.rb b/lib/roo/utils.rb index 8e8f7787..484d9190 100644 --- a/lib/roo/utils.rb +++ b/lib/roo/utils.rb @@ -4,7 +4,7 @@ module Utils LETTERS = ('A'..'Z').to_a - def extract_coord(s, klass = Excelx::Coordinate) + def extract_coordinate(s) num = letter_num = 0 num_only = false @@ -22,20 +22,16 @@ def extract_coord(s, klass = Excelx::Coordinate) end fail ArgumentError if letter_num == 0 || !num_only - if klass == Array - [num, letter_num] - else - klass.new(num, letter_num) - end + Excelx::Coordinate.new(num, letter_num) end - alias_method :extract_coordinate, :extract_coord + alias_method :ref_to_key, :extract_coordinate def split_coordinate(str) - extract_coord(str, Array) + warn "[DEPRECATION] `Roo::Utils.split_coordinate` is deprecated. Please use `Roo::Utils.extract_coordinate` instead." + extract_coordinate(str) end - alias_method :ref_to_key, :split_coordinate def split_coord(str) @@ -72,8 +68,8 @@ def num_cells_in_range(str) cells = str.split(':') return 1 if cells.count == 1 raise ArgumentError.new("invalid range string: #{str}. Supported range format 'A1:B2'") if cells.count != 2 - x1, y1 = split_coordinate(cells[0]) - x2, y2 = split_coordinate(cells[1]) + x1, y1 = extract_coordinate(cells[0]) + x2, y2 = extract_coordinate(cells[1]) (x2 - (x1 - 1)) * (y2 - (y1 - 1)) end diff --git a/spec/lib/roo/utils_spec.rb b/spec/lib/roo/utils_spec.rb index ffe93d42..7bfd11e8 100644 --- a/spec/lib/roo/utils_spec.rb +++ b/spec/lib/roo/utils_spec.rb @@ -52,6 +52,15 @@ end end + context '.extract_coordinate' do + it "returns the expected result" do + expect(described_class.extract_coordinate('A1')).to eq [1, 1] + expect(described_class.extract_coordinate('B2')).to eq [2, 2] + expect(described_class.extract_coordinate('R2')).to eq [2, 18] + expect(described_class.extract_coordinate('AR31')).to eq [31, 18 + 26] + end + end + context '.split_coord' do it "returns the expected result" do expect(described_class.split_coord('A1')).to eq ["A", 1] diff --git a/test/excelx/test_coordinate.rb b/test/excelx/test_coordinate.rb new file mode 100644 index 00000000..5cddcc40 --- /dev/null +++ b/test/excelx/test_coordinate.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestRooExcelxCoordinate < Minitest::Test + def row + 10 + end + + def column + 20 + end + + def coordinate + Roo::Excelx::Coordinate.new(row, column) + end + + def array + [row, column] + end + + def test_row + assert_same row, coordinate.row + end + + def test_column + assert_same column, coordinate.column + end + + def test_frozen? + assert coordinate.frozen? + end + + def test_equality + hash = {} + hash[coordinate] = true + assert hash.key?(coordinate) + assert hash.key?(array) + end + + def test_expand + r, c = coordinate + assert_same row, r + assert_same column, c + end + + def test_aref + assert_same row, coordinate[0] + assert_same column, coordinate[1] + end +end