fix data, labels, style draw() option handling to not set state in the

chart object

handle undef values introduced into $opts in _style_setup by the new
style accessors

diff --git a/Graph.pm b/Graph.pm
index 32606b2a305968f49e4334eea6b858303753d2c6..eb2f546ef45bebfe69bea1c7a0cc15c8dd2f976a 100644 (file)
--- a/Graph.pm
+++ b/Graph.pm
@@ -107,7 +107,21 @@ sub addDataSeries {
 }
 
 sub _getDataSeries {
-  return $_[0]->{'graph_data'};
+  my ($self, $opts) = @_;
+
+  # return the data supplied to draw() if any.
+  if ($opts->{data}) {
+    # one or multiple series?
+    my $data = $opts->{data};
+    if (@$data && ref $data->[0] && ref $data->[0] =~ /ARRAY/) {
+      return $data;
+    }
+    else {
+      return [ { data => $data } ];
+    }
+  }
+
+  return $self->{'graph_data'};
 }
 
 =item setLabels(['label1', 'label2' ... ])
@@ -121,6 +135,11 @@ sub setLabels {
 }
 
 sub _getLabels {
+  my ($self, $opts) = @_;
+
+  $opts->{labels}
+    and return $opts->{labels};
+
   return $_[0]->{'labels'}
 }
 
@@ -163,7 +182,12 @@ sub setStyle {
 }
 
 sub _getStyle {
-  return $_[0]->{'style'};
+  my ($self, $opts) = @_;
+
+  $opts->{style}
+    and return $opts->{style};
+
+  return $self->{'style'};
 }
 
 =item error
@@ -840,24 +864,6 @@ sub _style_defs {
   \%style_defs;
 }
 
-sub _processOptions {
-  my $self = shift;
-  my $opts = shift;
-
-  if ($opts->{'style'}) {
-    $self->setStyle($opts->{'style'});
-  }
-
-  if ($opts->{'data'}) {
-    $self->addDataSeries($opts->{'data'});
-  }
-  if ($opts->{'labels'}) {
-    $self->setLabels($opts->{'labels'});
-  }
-}
-
-
-
 # Let's make the default something that looks really good, so folks will be interested enough to customize the style.
 my $def_style = 'fount_lin';
 
@@ -1029,7 +1035,7 @@ sub _style_setup {
     $opts->{'title'} = { text => $self->_getTitle() };
   }
 
-  my $pre_def_style = $self->_getStyle();
+  my $pre_def_style = $self->_getStyle($opts);
   $style = $styles{$pre_def_style} if $pre_def_style;
 
   $style ||= $styles{$def_style};
@@ -1055,7 +1061,8 @@ sub _style_setup {
         }
       }
       else {
-        $work{$key} = $src->{$key};
+        $work{$key} = $src->{$key}
+         if defined $src->{$key}; # $opts with pmichauds new accessor handling
       }
     }
   }

diff --git a/lib/Imager/Graph/Pie.pm b/lib/Imager/Graph/Pie.pm
index 0ecc1320e29f39e577ac0813bb1a46fa58664a27..fb0ea8bca91fa18f3718a99508f0d05d0a0ae691 100644 (file)
--- a/lib/Imager/Graph/Pie.pm
+++ b/lib/Imager/Graph/Pie.pm
@@ -193,16 +193,14 @@ suitable for monochrome output:
 sub draw {
   my ($self, %opts) = @_;
 
-  $self->_processOptions(\%opts);
+  my $data_series = $self->_getDataSeries(\%opts);
 
-  if (!$self->_validInput()) {
-    return;
-  }
-
-  my @data = @{$self->_getDataSeries()->[0]->{'data'}};
+  $self->_validInput($data_series)
+    or return;
 
-  my @labels = @{$self->_getLabels() || []};
+  my @data = @{$data_series->[0]->{'data'}};
 
+  my @labels = @{$self->_getLabels(\%opts) || []};
 
   $self->_style_setup(\%opts);
 
@@ -430,22 +428,25 @@ sub draw {
 }
 
 sub _validInput {
-  my $self = shift;
+  my ($self, $data_series) = @_;
 
-  if (!defined $self->_getDataSeries() || !scalar @{$self->_getDataSeries()}) {
+  if (!defined $data_series || !scalar @$data_series) {
     return $self->_error("No data supplied");
   }
 
-  if (!scalar @{$self->_getDataSeries()->[0]->{'data'}}) {
+  @$data_series == 1
+    or return $self->_error("Pie charts only allow one data series");
+
+  my $data = $data_series->[0]{data};
+
+  if (!scalar @$data) {
     return $self->_error("No values in data series");
   }
 
-  my @data = @{$self->_getDataSeries()->[0]->{'data'}};
-
   my $total = 0;
   {
     my $index = 0;
-    for my $item (@data) {
+    for my $item (@$data) {
       $item < 0
         and return $self->_error("Data index $index is less than zero");
 

diff --git a/t/t10pie.t b/t/t10pie.t
index 21341f946d47297b0e35661112317cff456f8ab4..f57732b9c56c49b98ebccdf78b98adf55dc3851d 100644 (file)
--- a/t/t10pie.t
+++ b/t/t10pie.t
@@ -21,7 +21,7 @@ my $font = Imager::Font::Test->new();
 my @data = ( 100, 180, 80, 20, 2, 1, 0.5 );
 my @labels = qw(alpha beta gamma delta epsilon phi gi);
 
-plan tests => 34;
+plan tests => 35;
 
 my $pie = Imager::Graph::Pie->new;
 ok($pie, "creating pie chart object");
@@ -115,6 +115,12 @@ cmpimg($img6, "testimg/t10_hlegend.png", 550_000);
   # zero sized segments were drawn to cover the whole circle
   my @data = ( 10, 8, 5, 0.000 );
   my @labels = qw(alpha beta gamma);
+  my @warned;
+  local $SIG{__WARN__} = 
+    sub { 
+      print STDERR $_[0];
+      push @warned, $_[0]
+    };
   
   my $img = $pie->draw
     (
@@ -127,6 +133,9 @@ cmpimg($img6, "testimg/t10_hlegend.png", 550_000);
   ok($img->write(file => 'testout/t10_noother.ppm'),
      "save it");
   cmpimg($img, 'testimg/t10_noother.png', 500_000);
+  unless (is(@warned, 0, "should be no warnings")) {
+    diag($_) for @warned;
+  }
 }
 
 { # RT #535