Merge pull request #76 from ochko/master

Shared string table works!

7 files changed, 49 insertions, 41 deletions

diff --git a/.gitignore b/.gitignore
index 1c698903..29af6b25 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,6 +5,7 @@ coverage
 .yardoc
 *.gem
 *.xlsx
+example.csv
 *.*~
 .DS_Store
 tmp
\ No newline at end of file
diff --git a/lib/axlsx/stylesheet/color.rb b/lib/axlsx/stylesheet/color.rb
index 78171607..065eeb46 100644
--- a/lib/axlsx/stylesheet/color.rb
+++ b/lib/axlsx/stylesheet/color.rb
@@ -71,6 +71,5 @@ module Axlsx
       end
       str << "/>"
     end
-
   end
 end
diff --git a/lib/axlsx/workbook/shared_strings_table.rb b/lib/axlsx/workbook/shared_strings_table.rb
index 45402f5b..61402aec 100644
--- a/lib/axlsx/workbook/shared_strings_table.rb
+++ b/lib/axlsx/workbook/shared_strings_table.rb
@@ -29,11 +29,12 @@ module Axlsx
     # Creates a new Shared Strings Table agains an array of cells
     # @param [Array] cells This is an array of all of the cells in the workbook
     def initialize(cells)
-      cells = cells.flatten.reject { |c| c.type != :string || c.value.nil? || c.value.start_with?('=') }
-      @count = cells.size
-      @unique_cells = []
+      @index = 0
+      @unique_cells = {}
       @shared_xml_string = ""
-      resolve(cells)
+      shareable_cells = cells.flatten.select{ |cell| cell.plain_string? }
+      @count = shareable_cells.size
+      resolve(shareable_cells)
     end
 
     # Serializes the object
@@ -52,16 +53,14 @@ module Axlsx
     # @return [Array] unique cells
     def resolve(cells)
       cells.each do |cell|
-        cell_hash = cell.shareable_hash
-        index = @unique_cells.index do |item|
-          item == cell_hash
-        end
-        if index == nil
-          cell.send :ssti=, @unique_cells.size
-          @shared_xml_string << '<si>' << cell.run_xml_string << '</si>'
-          @unique_cells << cell_hash
-        else
+        cell_hash = cell.value
+        if index = @unique_cells[cell_hash]
           cell.send :ssti=, index
+        else
+          cell.send :ssti=, @index
+          @shared_xml_string << '<si>' << cell.run_xml_string << '</si>'
+          @unique_cells[cell_hash] = @index
+          @index += 1
         end
       end
     end
diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb
index f4c6ff09..6979a6d1 100644
--- a/lib/axlsx/workbook/worksheet/cell.rb
+++ b/lib/axlsx/workbook/worksheet/cell.rb
@@ -74,13 +74,20 @@ module Axlsx
       @value = cast_value(v)
     end
 
-
     # Indicates that the cell has one or more of the custom cell styles applied.
     # @return [Boolean]
     def is_text_run?
       @is_text_run ||= false
     end
 
+    # Indicates if the cell is good for shared string table
+    def plain_string?
+      @type == :string &&         # String typed
+        !@is_text_run &&          # No inline styles
+        [email protected]? &&           # Not nil
+        [email protected]? &&         # Not empty
+        [email protected]_with?('=')  # Not a formula
+    end
 
     # The inline font_name property for the cell
     # @return [String]
@@ -154,7 +161,7 @@ module Axlsx
     # @param [String] The 8 character representation for an rgb color #FFFFFFFF"
     def color=(v)
       @color = v.is_a?(Color) ? v : Color.new(:rgb=>v)
-      @has_run_style = true
+      @is_text_run = true
     end
 
     # The inline sz property for the cell
@@ -220,15 +227,6 @@ module Axlsx
     # @return [Integer]
     attr_reader :ssti
 
-    # equality comparison to test value, type and inline style attributes
-    # this is how we work out if the cell needs to be added or already exists in the shared strings table
-    def shareable_hash
-      self_hash = {}
-      INLINE_STYLES.each { |style| self_hash[style] = self.instance_variable_get("@" + style) }
-      self_hash['color'] = self_hash['color'].instance_values if self_hash['color']
-      self_hash
-    end
-
     # @return [Integer] The index of the cell in the containing row.
     def index
       @row.cells.index(self)
diff --git a/test/benchmark.rb b/test/benchmark.rb
index bb9a7b7d..a2179a24 100644
--- a/test/benchmark.rb
+++ b/test/benchmark.rb
@@ -24,7 +24,7 @@ Benchmark.bm(100) do |x|
         sheet << row
       end
     end
-    p.serialize("example.xlsx")
+    p.serialize("example_noautowidth.xlsx")
   }
 
   x.report('axlsx') {
@@ -38,7 +38,7 @@ Benchmark.bm(100) do |x|
         sheet << row
       end
     end
-    p.serialize("example.xlsx")
+    p.serialize("example_autowidth.xlsx")
   }
 
   x.report('axlsx_shared') {
@@ -53,7 +53,7 @@ Benchmark.bm(100) do |x|
       end
     end
     p.use_shared_strings = true
-    p.serialize("example.xlsx")
+    p.serialize("example_shared.xlsx")
   }
 
   x.report('axlsx_stream') {
diff --git a/test/profile.rb b/test/profile.rb
index 97d2e2bc..fa81577b 100644
--- a/test/profile.rb
+++ b/test/profile.rb
@@ -15,11 +15,12 @@ require 'perftools'
 row = []
 input = (32..126).to_a.pack('U*').chars.to_a
 20.times { row << input.shuffle.join}
-times = 1000
+times = 3000
 
 PerfTools::CpuProfiler.start("/tmp/axlsx_noautowidth") do
   p = Axlsx::Package.new
   p.use_autowidth = false
+  p.use_shared_strings = true
   wb = p.workbook
   
   #A Simple Workbook
diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb
index 1cfd6169..12b4fb4c 100644
--- a/test/workbook/worksheet/tc_cell.rb
+++ b/test/workbook/worksheet/tc_cell.rb
@@ -212,24 +212,34 @@ class TestCell < Test::Unit::TestCase
     assert_equal(@c.row.worksheet.merged_cells.last, "A1:C1")
   end
 
-  def test_equality
-    c2 = @row.add_cell 1, :type=>:float, :style=>1
-
-    assert_equal(c2.shareable_hash,@c.shareable_hash)
-      c3 = @row.add_cell 2, :type=>:float, :style=>1
-      c4 = @row.add_cell 1, :type=>:float, :style=>1, :color => "#FFFFFFFF"
-      assert_equal(c4.shareable_hash == c2.shareable_hash,false)
-      c5 = @row.add_cell 1, :type=>:float, :style=>1, :color => "#FFFFFFFF"
-      assert_equal(c5.shareable_hash, c4.shareable_hash)
-
-  end
-
   def test_ssti
     assert_raise(ArgumentError, "ssti must be an unsigned integer!") { @c.send(:ssti=, -1) }
     @c.send :ssti=, 1
     assert_equal(@c.ssti, 1)
   end
 
+  def test_plain_string
+    @c.type = :integer
+    assert_equal(@c.plain_string?, false)
+
+    @c.type = :string
+    @c.value = 'plain string'
+    assert_equal(@c.plain_string?, true)
+
+    @c.value = nil
+    assert_equal(@c.plain_string?, false)
+
+    @c.value = ''
+    assert_equal(@c.plain_string?, false)
+
+    @c.value = '=sum'
+    assert_equal(@c.plain_string?, false)
+
+    @c.value = 'plain string'
+    @c.font_name = 'Arial'
+    assert_equal(@c.plain_string?, false)
+  end
+
   def test_to_xml_string
     c_xml = Nokogiri::XML(@c.to_xml_string(1,1))
     assert_equal(c_xml.xpath("/c[@s=1]").size, 1)