refactoring

5 files changed, 105 insertions, 61 deletions

diff --git a/lib/Spreadsheet/Template/Generator.pm b/lib/Spreadsheet/Template/Generator.pm
index aa5e0e0..27c03ec 100644
--- a/lib/Spreadsheet/Template/Generator.pm
+++ b/lib/Spreadsheet/Template/Generator.pm
@@ -10,8 +10,8 @@ sub generate {
     (my $ext = $filename) =~ s/.*\.//;
     my $class = $self->parser_classes->{$ext};
     load_class($class);
-    my $parser = $class->new;
-    my $data = $parser->parse($filename);
+    my $parser = $class->new(filename => $filename);
+    my $data = $parser->parse;
     return JSON->new->pretty->encode($data);
 }
 
diff --git a/lib/Spreadsheet/Template/Generator/Parser.pm b/lib/Spreadsheet/Template/Generator/Parser.pm
index a2cfd25..fb3c272 100644
--- a/lib/Spreadsheet/Template/Generator/Parser.pm
+++ b/lib/Spreadsheet/Template/Generator/Parser.pm
@@ -3,6 +3,12 @@ use Moose::Role;
 
 requires 'parse';
 
+has filename => (
+    is       => 'ro',
+    isa      => 'Str',
+    required => 1,
+);
+
 no Moose::Role;
 
 1;
diff --git a/lib/Spreadsheet/Template/Generator/Parser/Excel.pm b/lib/Spreadsheet/Template/Generator/Parser/Excel.pm
index f69c368..c9a128a 100644
--- a/lib/Spreadsheet/Template/Generator/Parser/Excel.pm
+++ b/lib/Spreadsheet/Template/Generator/Parser/Excel.pm
@@ -3,25 +3,28 @@ use Moose::Role;
 
 with 'Spreadsheet::Template::Generator::Parser';
 
-requires 'make_excel';
+requires '_build_excel';
+
+has excel => (
+    is      => 'ro',
+    isa     => 'Object',
+    lazy    => 1,
+    builder => '_build_excel',
+);
 
 sub parse {
     my $self = shift;
-    my ($filename) = @_;
-
-    my $excel = $self->make_excel($filename);
-    return $self->_parse_workbook($excel);
+    return $self->_parse_workbook;
 }
 
 sub _parse_workbook {
     my $self = shift;
-    my ($excel) = @_;
 
     my $data = {
         worksheets => [],
     };
 
-    for my $sheet ($excel->worksheets) {
+    for my $sheet ($self->excel->worksheets) {
         push @{ $data->{worksheets} }, $self->_parse_worksheet($sheet);
     }
 
diff --git a/lib/Spreadsheet/Template/Generator/Parser/XLS.pm b/lib/Spreadsheet/Template/Generator/Parser/XLS.pm
index eb5c949..194558a 100644
--- a/lib/Spreadsheet/Template/Generator/Parser/XLS.pm
+++ b/lib/Spreadsheet/Template/Generator/Parser/XLS.pm
@@ -5,29 +5,45 @@ use Spreadsheet::ParseExcel;
 
 with 'Spreadsheet::Template::Generator::Parser::Excel';
 
-sub make_excel {
+sub _build_excel {
     my $self = shift;
-    my ($filename) = @_;
 
     my $parser = Spreadsheet::ParseExcel->new;
-    my $excel = $parser->parse($filename);
+    my $excel = $parser->parse($self->filename);
     die $parser->error unless $excel;
 
-    # just for consistency
+    $self->_fixup_excel($excel);
+
+    return $excel;
+}
+
+sub _fixup_excel {
+    my $self = shift;
+    my ($excel) = @_;
+
     for my $sheet ($excel->worksheets) {
-        $sheet->{RowHeight} = [
-            map { defined $_ ? $_ : $sheet->get_default_row_height }
-                $sheet->get_row_heights
-        ];
-        $sheet->{ColWidth} = [
-            map { defined $_ ? $_ : $sheet->get_default_col_width }
-                $sheet->get_col_widths
-        ];
+        $self->_normalize_cell_sizes($sheet);
+        $self->_parse_formulas($sheet);
     }
+}
 
-    # XXX no formula support yet
+sub _normalize_cell_sizes {
+    my $self = shift;
+    my ($sheet) = @_;
 
-    return $excel;
+    # just for consistency
+    $sheet->{RowHeight} = [
+        map { defined $_ ? $_ : $sheet->get_default_row_height }
+            $sheet->get_row_heights
+    ];
+    $sheet->{ColWidth} = [
+        map { defined $_ ? $_ : $sheet->get_default_col_width }
+            $sheet->get_col_widths
+    ];
+}
+
+sub _parse_formulas {
+    # XXX no formula support yet
 }
 
 __PACKAGE__->meta->make_immutable;
diff --git a/lib/Spreadsheet/Template/Generator/Parser/XLSX.pm b/lib/Spreadsheet/Template/Generator/Parser/XLSX.pm
index cf7c997..c9d12a1 100644
--- a/lib/Spreadsheet/Template/Generator/Parser/XLSX.pm
+++ b/lib/Spreadsheet/Template/Generator/Parser/XLSX.pm
@@ -8,12 +8,20 @@ use XML::Twig;
 
 with 'Spreadsheet::Template::Generator::Parser::Excel';
 
-sub make_excel {
+sub _build_excel {
     my $self = shift;
-    my ($filename) = @_;
-    my $excel = Spreadsheet::XLSX->new($filename);
+    my $excel = Spreadsheet::XLSX->new($self->filename);
+    $self->_fixup_excel($excel);
+    return $excel;
+}
+
+# XXX Spreadsheet::XLSX doesn't extract this information currently
+sub _fixup_excel {
+    my $self = shift;
+    my ($excel) = @_;
+
+    my $filename = $self->filename;
 
-    # XXX Spreadsheet::XLSX doesn't extract this information currently
     my $zip = Archive::Zip->new;
     die "Can't open $filename as zip file"
         unless $zip->read($filename) == Archive::Zip::AZ_OK;
@@ -22,46 +30,57 @@ sub make_excel {
         my $contents = $zip->memberNamed("xl/$sheet->{path}")->contents;
         next unless $contents;
 
-        my @column_widths;
-        my @row_heights;
-
         my $xml = XML::Twig->new;
         $xml->parse($contents);
         my $root = $xml->root;
 
-        my ($format) = $root->find_nodes('//sheetFormatPr');
-        my $default_row_height = $format->att('defaultRowHeight');
-        my $default_column_width = $format->att('baseColWidth');
-
-        for my $col ($root->find_nodes('//col')) {
-            $column_widths[$col->att('min') - 1] = $col->att('width');
-        }
-
-        for my $row ($root->find_nodes('//row')) {
-            $row_heights[$row->att('r') - 1] = $row->att('ht');
-        }
-
-        $sheet->{DefRowHeight} = 0+$default_row_height;
-        $sheet->{DefColWidth} = 0+$default_column_width;
-        $sheet->{RowHeight} = [
-            map { defined $_ ? 0+$_ : 0+$default_row_height } @row_heights
-        ];
-        $sheet->{ColWidth} = [
-            map { defined $_ ? 0+$_ : 0+$default_column_width } @column_widths
-        ];
-
-        for my $formula ($root->find_nodes('//f')) {
-            my $cell_id = $formula->parent->att('r');
-            my ($col, $row) = $cell_id =~ /([A-Z]+)([0-9]+)/;
-            $col =~ tr/A-Z/0-9A-P/;
-            $col = POSIX::strtol($col, 26);
-            $row = $row - 1;
-            my $cell = $sheet->get_cell($row, $col);
-            $cell->{Formula} = "=" . $formula->text;
-        }
+        $self->_parse_cell_sizes($sheet, $root);
+        $self->_parse_formulas($sheet, $root);
     }
+}
 
-    return $excel;
+sub _parse_cell_sizes {
+    my $self = shift;
+    my ($sheet, $root) = @_;
+
+    my @column_widths;
+    my @row_heights;
+
+    my ($format) = $root->find_nodes('//sheetFormatPr');
+    my $default_row_height = $format->att('defaultRowHeight');
+    my $default_column_width = $format->att('baseColWidth');
+
+    for my $col ($root->find_nodes('//col')) {
+        $column_widths[$col->att('min') - 1] = $col->att('width');
+    }
+
+    for my $row ($root->find_nodes('//row')) {
+        $row_heights[$row->att('r') - 1] = $row->att('ht');
+    }
+
+    $sheet->{DefRowHeight} = 0+$default_row_height;
+    $sheet->{DefColWidth} = 0+$default_column_width;
+    $sheet->{RowHeight} = [
+        map { defined $_ ? 0+$_ : 0+$default_row_height } @row_heights
+    ];
+    $sheet->{ColWidth} = [
+        map { defined $_ ? 0+$_ : 0+$default_column_width } @column_widths
+    ];
+}
+
+sub _parse_formulas {
+    my $self = shift;
+    my ($sheet, $root) = @_;
+
+    for my $formula ($root->find_nodes('//f')) {
+        my $cell_id = $formula->parent->att('r');
+        my ($col, $row) = $cell_id =~ /([A-Z]+)([0-9]+)/;
+        $col =~ tr/A-Z/0-9A-P/;
+        $col = POSIX::strtol($col, 26);
+        $row = $row - 1;
+        my $cell = $sheet->get_cell($row, $col);
+        $cell->{Formula} = "=" . $formula->text;
+    }
 }
 
 # XXX this stuff all feels like working around bugs in Spreadsheet::XLSX -