Code review changes from drgrice1.

Mostly unpacking function calls, blessing objects in a single line, fixing perl calls, along with other code cleanup suggestions.
openwebwork · Aug 15, 2024 · be70a50 · be70a50
1 parent 922a65f
commit be70a50
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 74 deletions.
diff --git a/lib/Plots/Axes.pm b/lib/Plots/Axes.pm
@@ -195,7 +195,7 @@ use warnings;
 
 sub new {
 	my $class = shift;
-	my $self  = {
+	my $self  = bless {
 		xaxis  => {},
 		yaxis  => {},
 		styles => {
@@ -206,9 +206,8 @@ sub new {
 			show_grid  => 1,
 		},
 		@_
-	};
+	}, $class;
 
-	bless $self, $class;
 	$self->xaxis($self->axis_defaults('x'));
 	$self->yaxis($self->axis_defaults('y'));
 	return $self;
@@ -249,13 +248,13 @@ sub axis {
 }
 
 sub xaxis {
-	my $self = shift;
-	return $self->axis('xaxis', @_);
+	my ($self, @items) = @_;
+	return $self->axis('xaxis', @items);
 }
 
 sub yaxis {
-	my $self = shift;
-	return $self->axis('yaxis', @_);
+	my ($self, @items) = @_;
+	return $self->axis('yaxis', @items);
 }
 
 sub set {

diff --git a/lib/Plots/Data.pm b/lib/Plots/Data.pm
@@ -111,18 +111,8 @@ use strict;
 use warnings;
 
 sub new {
-	my $class = shift;
-	my $self  = {
-		name     => '',
-		x        => [],
-		y        => [],
-		function => {},
-		styles   => {},
-		@_
-	};
-
-	bless $self, $class;
-	return $self;
+	my ($class, %options) = @_;
+	return bless { name => '', x => [], y => [], function => {}, styles => {}, %options }, $class;
 }
 
 sub name {
@@ -166,13 +156,13 @@ sub style {
 }
 
 sub set_function {
-	my $self = shift;
+	my ($self, %options) = @_;
 	$self->{function} = {
 		sub_x => sub { return $_[0]; },
 		sub_y => sub { return $_[0]; },
 		min   => -5,
 		max   => 5,
-		@_
+		%options
 	};
 	$self->style(steps => $self->{function}{steps}) if $self->{funciton}{steps};
 	return;
@@ -211,11 +201,11 @@ sub _add {
 }
 
 sub add {
-	my $self = shift;
-	if (ref($_[0]) eq 'ARRAY') {
-		for (@_) { $self->_add(@$_); }
+	my ($self, @points) = @_;
+	if (ref($points[0]) eq 'ARRAY') {
+		for (@points) { $self->_add(@$_); }
 	} else {
-		$self->_add(@_);
+		$self->_add(@points);
 	}
 	return;
 }

diff --git a/lib/Plots/GD.pm b/lib/Plots/GD.pm
@@ -30,16 +30,13 @@ use warnings;
 
 sub new {
 	my ($class, $pgplot) = @_;
-	my $self = {
+	return bless {
 		image    => '',
 		pgplot   => $pgplot,
 		position => [ 0, 0 ],
 		colors   => {},
-	};
-	bless $self, $class;
-
-	$self->{image} = new GD::Image($pgplot->size);
-	return $self;
+		image    => GD::Image->new($pgplot->size)
+	}, $class;
 }
 
 sub pgplot {
@@ -199,7 +196,7 @@ sub draw_label {
 
 sub draw_arrow_head {
 	my ($self, $x1, $y1, $x2, $y2, $color, $w) = @_;
-	return unless scalar(@_) > 4;
+	return unless @_ > 4;
 	$color = $self->color($color || 'default_color');
 	$w     = 1 unless $w;
 	($x1, $y1) = ($self->im_x($x1), $self->im_y($y1));
@@ -214,7 +211,7 @@ sub draw_arrow_head {
 	my $py   = $ux;
 	my $hbx  = $x2 - 7 * $w * $ux;
 	my $hby  = $y2 - 7 * $w * $uy;
-	my $head = new GD::Polygon;
+	my $head = GD::Polygon->new;
 	$head->addPt($x2,                 $y2);
 	$head->addPt($hbx + 3 * $w * $px, $hby + 3 * $w * $py);
 	$head->addPt($hbx - 3 * $w * $px, $hby - 3 * $w * $py);

diff --git a/lib/Plots/Plot.pm b/lib/Plots/Plot.pm
@@ -32,10 +32,10 @@ use Plots::Tikz;
 use Plots::GD;
 
 sub new {
-	my ($class, $pg, @opts) = @_;
+	my ($class, $pg, %options) = @_;
 	my $size = $main::envir{onTheFlyImageSize} || 500;
 
-	my $self = {
+	my $self = bless {
 		pg        => $pg,
 		imageName => {},
 		type      => 'Tikz',
@@ -44,10 +44,9 @@ sub new {
 		axes      => Plots::Axes->new,
 		colors    => {},
 		data      => [],
-		@opts
-	};
+		%options
+	}, $class;
 
-	bless $self, $class;
 	$self->color_init;
 	return $self;
 }
@@ -57,18 +56,12 @@ sub colors {
 	return defined($color) ? $self->{colors}{$color} : $self->{colors};
 }
 
-sub _add_color {
-	my ($self, $color, $r, $g, $b) = @_;
-	$self->{'colors'}{$color} = [ $r, $g, $b ];
-	return;
-}
-
 sub add_color {
-	my $self = shift;
-	if (ref($_[0]) eq 'ARRAY') {
-		for (@_) { $self->_add_color(@$_); }
+	my ($self, @colors) = @_;
+	if (ref($colors[0]) eq 'ARRAY') {
+		for (@colors) { $self->{colors}{ $_->[0] } = [ @$_[ 1 .. 3 ] ]; }
 	} else {
-		$self->_add_color(@_);
+		$self->{colors}{ $colors[0] } = [ @colors[ 1 .. 3 ] ];
 	}
 	return;
 }
@@ -98,7 +91,10 @@ sub size {
 sub data {
 	my ($self, @names) = @_;
 	return wantarray ? @{ $self->{data} } : $self->{data} unless @names;
-	my @data = grep { my $name = $_->name; grep(/^$name$/, @names) } @{ $self->{data} };
+	my @data = grep {
+		my $name = $_->name;
+		grep {/^$name$/} @names
+	} @{ $self->{data} };
 	return wantarray ? @data : \@data;
 }
 
@@ -161,15 +157,15 @@ sub image_type {
 # Tikz needs to use pdf for hardcopy generation.
 sub ext {
 	my $self = shift;
-	return 'pdf' if ($self->{type} eq 'Tikz' && $main::displayMode eq 'TeX');
+	return 'pdf' if ($self->{type} eq 'Tikz' && eval('$main::displayMode') eq 'TeX');
 	return $self->{ext};
 }
 
 # Return a copy of the tikz code (available after the image has been drawn).
 # Set $plot->{tikzDebug} to 1 to just generate the tikzCode, and not create a graph.
 sub tikz_code {
 	my $self = shift;
-	return ($self->{tikzCode} && $main::displayMode =~ /HTML/) ? '<pre>' . $self->{tikzCode} . '</pre>' : '';
+	return ($self->{tikzCode} && eval('$main::displayMode') =~ /HTML/) ? '<pre>' . $self->{tikzCode} . '</pre>' : '';
 }
 
 # Add functions to the graph.
@@ -298,11 +294,11 @@ sub _add_dataset {
 }
 
 sub add_dataset {
-	my $self = shift;
-	if (ref($_[0]) eq 'ARRAY' && ref($_[0]->[0]) eq 'ARRAY') {
-		return [ map { $self->_add_dataset(@$_); } @_ ];
+	my ($self, @data) = @_;
+	if (ref($data[0]) eq 'ARRAY' && ref($data[0][0]) eq 'ARRAY') {
+		return [ map { $self->_add_dataset(@$_); } @data ];
 	}
-	return $self->_add_dataset(@_);
+	return $self->_add_dataset(@data);
 }
 
 sub _add_label {
@@ -324,8 +320,8 @@ sub _add_label {
 }
 
 sub add_label {
-	my $self = shift;
-	return ref($_[0]) eq 'ARRAY' ? [ map { $self->_add_label(@$_); } @_ ] : $self->_add_label(@_);
+	my ($self, @labels) = @_;
+	return ref($labels[0]) eq 'ARRAY' ? [ map { $self->_add_label(@$_); } @labels ] : $self->_add_label(@labels);
 }
 
 # Fill regions only work with GD and are ignored in TikZ images.
@@ -339,8 +335,11 @@ sub _add_fill_region {
 }
 
 sub add_fill_region {
-	my $self = shift;
-	return ref($_[0]) eq 'ARRAY' ? [ map { $self->_add_fill_region(@$_); } @_ ] : $self->_add_fill_region(@_);
+	my ($self, @regions) = @_;
+	return
+		ref($regions[0]) eq 'ARRAY'
+		? [ map { $self->_add_fill_region(@$_); } @regions ]
+		: $self->_add_fill_region(@regions);
 }
 
 sub _add_stamp {
@@ -358,8 +357,8 @@ sub _add_stamp {
 }
 
 sub add_stamp {
-	my $self = shift;
-	return ref($_[0]) eq 'ARRAY' ? [ map { $self->_add_stamp(@$_); } @_ ] : $self->_add_stamp(@_);
+	my ($self, @stamps) = @_;
+	return ref($stamps[0]) eq 'ARRAY' ? [ map { $self->_add_stamp(@$_); } @stamps ] : $self->_add_stamp(@stamps);
 }
 
 # Output the image based on a configurable type:

diff --git a/lib/Plots/Tikz.pm b/lib/Plots/Tikz.pm
@@ -28,23 +28,16 @@ use warnings;
 
 sub new {
 	my ($class, $pgplot) = @_;
-	my $image = new LaTeXImage;
+	my $image = LaTeXImage->new;
 	$image->environment('tikzpicture');
-	$image->svgMethod($main::envir{latexImageSVGMethod}           // 'pdf2svg');
+	$image->svgMethod($main::envir{latexImageSVGMethod}           // 'dvisvgm');
 	$image->convertOptions($main::envir{latexImageConvertOptions} // { input => {}, output => {} });
 	$image->ext($pgplot->ext);
-	$image->tikzLibraries('arrows.meta');
+	$image->tikzLibraries('arrows.meta,plotmarks');
 	$image->texPackages(['pgfplots']);
 	$image->addToPreamble('\pgfplotsset{compat=1.18}\usepgfplotslibrary{fillbetween}');
 
-	my $self = {
-		image  => $image,
-		pgplot => $pgplot,
-		colors => {},
-	};
-	bless $self, $class;
-
-	return $self;
+	return bless { image => $image, pgplot => $pgplot, colors => {} }, $class;
 }
 
 sub pgplot {
@@ -79,8 +72,8 @@ sub configure_axes {
 	my $ymajor      = $show_grid && $grid->{ymajor} ? 'true'          : 'false';
 	my $yminor_num  = $show_grid && $grid->{ymajor} ? $grid->{yminor} : 0;
 	my $yminor      = $yminor_num > 0 ? 'true' : 'false';
-	my $xticks      = join(',', @{ $grid->{xticks} });
-	my $yticks      = join(',', @{ $grid->{yticks} });
+	my $xticks      = '',#join(',', @{ $grid->{xticks} });
+	my $yticks      = '',#join(',', @{ $grid->{yticks} });
 	my $grid_color  = $axes->style('grid_color');
 	my $grid_color2 = $self->get_color($grid_color);
 	my $grid_alpha  = $axes->style('grid_alpha');