From f08b33a9122aa908bb49a513b0ccd29c3c2b7562 Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Fri, 26 Nov 2010 16:31:35 +0000
Subject: [PATCH] Bug 5448: Refactor Boolean.pm

Remove unnecesssary export
Use Exporter according to best practices
Use Carp not warn so we know where the error string came from
Don't create warnings manipulating undefined input
Replace package variables by lexicals

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
---
 C4/Boolean.pm | 69 +++++++++++++++++++--------------------------------
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/C4/Boolean.pm b/C4/Boolean.pm
index f546464cf0..02c0014b84 100644
--- a/C4/Boolean.pm
+++ b/C4/Boolean.pm
@@ -24,26 +24,15 @@ package C4::Boolean;
 use strict;
 use warnings;
 
-use POSIX;
-
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
-
-BEGIN {
-	# set the version for version checking
-	$VERSION = 0.02;
-	require Exporter;
-	@EXPORT = qw(
-		&INVALID_BOOLEAN_STRING_EXCEPTION
-    );
-	@EXPORT_OK = qw(
-		true_p
-    );
-	@ISA = qw(Exporter);
-}
+use Carp;
+use base qw(Exporter);
+
+our    $VERSION = 0.03;
+our    @EXPORT_OK = qw( true_p);
 
 =head1 NAME
 
-C4::Boolean - Convenience functions to handle boolean values
+C4::Boolean - Convenience function to handle boolean values
 in the parameter table
 
 =head1 SYNOPSIS
@@ -63,25 +52,23 @@ Boolean values in a consistent way which makes common sense.
 
 =cut
 
-sub INVALID_BOOLEAN_STRING_EXCEPTION ()
-    { 'The given value does not seem to be interpretable as a Boolean value' }
-
-use vars qw( %strings );
-
-%strings = (
-   '0'     => 0,	'1'     => 1,	# C
-   			'-1'    => 1,	# BASIC
-   'nil'   => 0,	't'     => 1,	# LISP
-   'false' => 0,	'true'  => 1,	# Pascal
-   'off'   => 0,	'on'    => 1,
-   'no'    => 0,	'yes'   => 1,
-   'n'     => 0,	'y'     => 1,
+use constant INVALID_BOOLEAN_STRING_EXCEPTION =>
+  q{The given value does not seem to be interpretable as a Boolean value};
+
+our %strings = (
+   '0'     => 0,    '1'     => 1,    # C
+                    '-1'    => 1,    # BASIC
+   'nil'   => 0,    't'     => 1,    # LISP
+   'false' => 0,    'true'  => 1,    # Pascal
+   'off'   => 0,    'on'    => 1,
+   'no'    => 0,    'yes'   => 1,
+   'n'     => 0,    'y'     => 1,
 );
 
 =item true_p
 
     if ( C4::Boolean::true_p(C4::Context->preference("insecure")) ) {
-	...
+    ...
     }
 
 Tries to interpret the passed string as a Boolean value. Returns
@@ -90,27 +77,23 @@ exception is thrown.
 
 =cut
 
-sub true_p ($) {
-    my($x) = @_;
+sub true_p  {
+    my $x = shift;
     my $it;
-    if (!defined $x || ref($x) ne '') {
-	warn INVALID_BOOLEAN_STRING_EXCEPTION;
+    if (!defined $x || ref $x ) {
+        carp INVALID_BOOLEAN_STRING_EXCEPTION;
+        return;
     }
-    $x = lc($x);
+    $x = lc $x;
     $x =~ s/\s//g;
     if (defined $strings{$x}) {
-	$it = $strings{$x};
+        $it = $strings{$x};
     } else {
-	warn INVALID_BOOLEAN_STRING_EXCEPTION;
+        carp INVALID_BOOLEAN_STRING_EXCEPTION;
     }
     return $it;
 }
 
-
-#---------------------------------
-
-END { }       # module clean-up code here (global destructor)
-
 1;
 __END__
 
-- 
2.39.5