Refactored nokogiri parser

.gitignore

@@ -8,3 +8,4 @@ doc/*
 log/*
 *.rbc
 .ruby-version
+coverage

lib/sheets/parsers/nokogiri_xlsx_parser.rb

@@ -5,39 +5,42 @@ class Sheets::Parsers::NokogiriXlsxParser < Sheets::Parsers::Base
   parses :xlsx
 
   def to_array
-    # Create Matrices
-    matrices = worksheets.collect do |worksheet|
-      worksheet.css('sheetData>row').collect do |row|
-        row.css('c').collect do |cell|
-          cell_value = cell.css('v').text
-
-          if cell.attribute('t')
-            celltype = cell.attribute('t').value
-            if celltype == 's'
-              # Load Shared String Value
-              cell_value = shared_strings[cell_value.to_i]
-            elsif celltype == 'b'
-              cell_value = (cell_value == "1") ? "TRUE" : "FALSE"
-            end
-          end
-
-          if cell.attribute('s') && cell.attribute('s').value == "1"
-            cell_value = (base_date + cell_value.to_f).strftime('%Y-%m-%d') # Date conversion
-          end
-
-          if cell_value.match(/\A[0-9]+\.?[0-9]*\Z/)
-            cell_value = cell_value.to_f.to_s
-          end
-
-          cell_value
-        end
-      end
+    extract_worksheet(worksheets.first)
+  end
+
+  private
+
+  def extract_worksheet(worksheet)
+    worksheet.css('sheetData>row').collect {|row| extract_row(row) }
+  end
+
+  def extract_row(row)
+    row.css('c').collect {|cell| value_for_cell(cell) }
+  end
+
+  def value_for_cell(cell)
+    cell_value = value_for_cell_type(cell.css('v').text, cell_type(cell))
+
+    if cell.attribute('s') && cell.attribute('s').value == "1"
+      cell_value = (base_date + cell_value.to_f).strftime('%Y-%m-%d') # Date conversion
+    end
+
+    if cell_value.match(/\A[0-9]+\.?[0-9]*\Z/)
+      cell_value = cell_value.to_f.to_s
     end
 
-    matrices.first
+    cell_value
   end
 
-  private
+  def cell_type(cell)
+    cell.attribute('t') ? cell.attribute('t').text : nil
+  end
+
+  def value_for_cell_type(cell_value, type)
+    { 's' => shared_strings[cell_value.to_i],
+      'b' => ((cell_value == "1") ? "TRUE" : "FALSE")
+    }[type] || cell_value
+  end
 
   # returns the zipfile object for the document
   def zipfile

sheets.gemspec

@@ -19,6 +19,7 @@ Gem::Specification.new do |s|
   s.add_dependency('nokogiri', '~>1.4.3.1')
 
   s.add_development_dependency('rake', '0.9.2')
+  s.add_development_dependency('simplecov')
 
   s.files         = `git ls-files`.split("\n")
   s.test_files    = `git ls-files -- {test,spec,features}/*`.split("\n")

test/parsers/nokogiri_xlsx_parser_test.rb

@@ -1,10 +1,25 @@
 class TestNokogiriXlsxParser < Test::Unit::TestCase
-  def setup
+  def test_does_not_raise_error_when_second_workbook_has_id_of_3
     file_path = File.expand_path("../../data/reordered_workbooks.xlsx", __FILE__)
-    @sheet = Sheets::Parsers::NokogiriXlsxParser.new(File.read(file_path), :xlsx, file_path)
+    sheet = Sheets::Parsers::NokogiriXlsxParser.new(File.read(file_path), :xlsx, file_path)
+    assert_nothing_raised { sheet.to_array }
   end
 
-  def test_does_not_raise_error_when_second_workbook_has_id_of_3
-    assert_nothing_raised { @sheet.to_array }
+  def test_boolean_cell_type
+    file_path = File.expand_path("../../data/simple_with_booleans.xlsx", __FILE__)
+    sheet = Sheets::Parsers::NokogiriXlsxParser.new(File.read(file_path), :xlsx, file_path)
+    assert_equal sheet.to_array, [
+      ["Date", "Impressions", "Clicks", "Actions", "Boolean"],
+      ["2011-01-01", "10.0", "10.0", "10.0", "TRUE"],
+      ["2011-01-02", "10.0", "10.0", "10.0", "TRUE"],
+      ["2011-01-03", "10.0", "10.0", "10.0", "FALSE"],
+      ["2011-01-04", "10.0", "10.0", "10.0", "FALSE"],
+      ["2011-01-05", "10.0", "10.0", "10.0", "TRUE"],
+      ["2011-01-06", "10.0", "10.0", "10.0", "TRUE"],
+      ["2011-01-07", "10.0", "10.0", "10.0", "FALSE"],
+      ["2011-01-08", "10.0", "10.0", "10.0", "FALSE"],
+      ["2011-01-09", "10.0", "10.0", "10.0", "TRUE"],
+      ["2011-01-10", "10.0", "10.0", "10.0", "TRUE"]
+    ]
   end
 end

test/test_helper.rb

@@ -1,6 +1,9 @@
+require 'simplecov'
+SimpleCov.start
+
 require 'test/unit'
 
 puts 'Loading Test Generators'
 Dir[ File.join(File.expand_path(File.dirname(__FILE__)), 'generators', '*.rb') ].each {|file| puts "- #{file}"; require file }
 
-require File.join(File.expand_path(File.dirname(__FILE__)), '..', 'lib', 'sheets.rb')
+require File.join(File.expand_path(File.dirname(__FILE__)), '..', 'lib', 'sheets.rb')