From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Wed, 12 Jan 2011 16:53:22 +0000 (+0000)
Subject: Bug 5611 Tidy up C4::Scrubber
X-Git-Url: http://git.rot13.org/?a=commitdiff_plain;h=073fb1aee655f9f29e99cc56ddf584eb6c7813a9;p=koha.git

Bug 5611 Tidy up C4::Scrubber

Remove the unneeded package variables
Remove unnecessary variables
Removed some cargo-cult programming
Tidied the indentation from mix of tabs & spaces
Add a Test that we actually return the required class
of object

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
---

diff --git a/C4/Scrubber.pm b/C4/Scrubber.pm
index d01795aa13..854812ef21 100644
--- a/C4/Scrubber.pm
+++ b/C4/Scrubber.pm
@@ -22,42 +22,36 @@ use HTML::Scrubber;
 use C4::Context;
 use C4::Debug;
 
-use vars qw($VERSION @ISA);
-use vars qw(%scrubbertypes $scrubbertype);
-
-BEGIN {
-	$VERSION = 0.02;
-	# @ISA = qw(HTML::Scrubber);
-}
+our $VERSION = 0.02;
 
 
 my %scrubbertypes = (
-	default => {},	# place holder, default settings are below as fallbacks in call to constructor
-	    tag => {},	# uses defaults
-	comment => {
-	allow   => [qw( br b i em big small strong )],
-	},
-	staff   => {
-	default => [ 1 =>{'*'=>1} ],
-	comment => 1,
-	},
+    default => {}, # place holder, default settings are below as fallbacks in call to constructor
+    tag     => {},                                               # uses defaults
+    comment => { allow => [qw( br b i em big small strong )], },
+    staff   => {
+        default => [ 1 => { '*' => 1 } ],
+        comment => 1,
+    },
 );
 
 
 sub new {
-	my $fakeself = shift;	# not really OO, we return an HTML::Scrubber object.
-	my $type  = (@_) ? shift : 'default';
-	exists $scrubbertypes{$type} or croak "New called with unrecognized type '$type'";
-	$debug and print STDERR "Building new Scrubber of type '$type'\n";
-	my $settings = $scrubbertypes{$type};
-	my $scrubber = HTML::Scrubber->new(
-		allow   => exists $settings->{allow}   ? $settings->{allow}   : [],
-		rules   => exists $settings->{rules}   ? $settings->{rules}   : [],
-		default => exists $settings->{default} ? $settings->{default} : [ 0 =>{'*'=>0} ],
-		comment => exists $settings->{comment} ? $settings->{comment} : 0,
-		process => 0,
-	);
-	return $scrubber;
+    shift; # ignore our class we are wraooer
+    my $type = (@_) ? shift : 'default';
+    if ( !exists $scrubbertypes{$type} ) {
+        croak "New called with unrecognized type '$type'";
+    }
+    $debug and carp "Building new Scrubber of type '$type'";
+    my $settings = $scrubbertypes{$type};
+    my $scrubber = HTML::Scrubber->new(
+        allow   => exists $settings->{allow} ? $settings->{allow} : [],
+        rules   => exists $settings->{rules} ? $settings->{rules} : [],
+        default => exists $settings->{default} ? $settings->{default} : [ 0 => { '*' => 0 } ],
+        comment => exists $settings->{comment} ? $settings->{comment} : 0,
+        process => 0,
+    );
+    return $scrubber;
 }
 
 
diff --git a/t/Scrubber.t b/t/Scrubber.t
index e0a2ad7ba9..ce1503f0ab 100755
--- a/t/Scrubber.t
+++ b/t/Scrubber.t
@@ -3,7 +3,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 10;
+use Test::More tests => 11;
 BEGIN {
 	use FindBin;
 	use lib $FindBin::Bin;
@@ -46,6 +46,9 @@ At the end here, I actually have some regular text.
 print pretty_line("Original HTML:"), $html, "\n", pretty_line();
 $collapse and diag "Note: scrubber test output will have whitespace collapsed for readability\n";
 ok($scrubber = C4::Scrubber->new(), "Constructor: C4::Scrubber->new()");
+
+isa_ok($scrubber, 'HTML::Scrubber', 'Constructor returns HTML::Scrubber object');
+
 ok(printf("# scrubber settings: default %s, comment %s, process %s\n",
 	$scrubber->default(),$scrubber->comment(),$scrubber->process()),
 	"Outputting settings from scrubber object (type: [default])"