Bug 8429: Remove unnecessary use of Exporter from SIP/ILS

All the modules in the SIP/ILS tree are objects
The addition of calls to Exporter or hand manipulation of
@ISA added unnecessary bloat
Removed the "self = shift or return" idiom  as it is nonsensical
if the method can only be called via an object.
standardized inheritance via use parent
added a $self = shift in a couple of places where it
was not strictly necessary as its absence seemed to have
misled readers in the past

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passed-QA-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

C4/SIP/ILS/Item.pm

@@ -22,14 +22,7 @@ use C4::Circulation;
 use C4::Members;
 use C4::Reserves;
 
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
-
-BEGIN {
-    $VERSION = 3.07.00.049;
-	require Exporter;
-	@ISA = qw(Exporter);
-	@EXPORT_OK = qw();
-}
+our $VERSION = 3.07.00.049;
 
 =head1 EXAMPLE
 
@@ -140,7 +133,7 @@ my %fields = (
 );
 
 sub next_hold {
-    my $self = shift or return;
+    my $self = shift;
     # use Data::Dumper; warn "next_hold() hold_shelf: " . Dumper($self->{hold_shelf}); warn "next_hold() pending_queue: " . $self->{pending_queue};
     foreach (@{$self->hold_shelf}) {    # If this item was taken from the hold shelf, then that reserve still governs
         next unless ($_->{itemnumber} and $_->{itemnumber} == $self->{itemnumber});
@@ -168,7 +161,7 @@ sub hold_patron_id {
 
 }
 sub hold_patron_name {
-    my $self = shift or return;
+    my $self = shift;
     my $borrowernumber = (@_ ? shift: $self->hold_patron_id()) or return;
     my $holder = GetMember(borrowernumber=>$borrowernumber);
     unless ($holder) {
@@ -186,7 +179,7 @@ sub hold_patron_name {
 }
 
 sub hold_patron_bcode {
-    my $self = shift or return;
+    my $self = shift;
     my $borrowernumber = (@_ ? shift: $self->hold_patron_id()) or return;
     my $holder = GetMember(borrowernumber => $borrowernumber);
     if ($holder) {
@@ -257,9 +250,11 @@ sub sip_circulation_status {
 }
 
 sub sip_security_marker {
+    my $self = shift;
     return '02';	# FIXME? 00-other; 01-None; 02-Tattle-Tape Security Strip (3M); 03-Whisper Tape (3M)
 }
 sub sip_fee_type {
+    my $self = shift;
     return '01';    # FIXME? 01-09 enumerated in spec.  We just use O1-other/unknown.
 }
 
@@ -354,7 +349,7 @@ sub _barcode_to_borrowernumber {
     return $member->{borrowernumber};
 }
 sub barcode_is_borrowernumber {    # because hold_queue only has borrowernumber...
-    my $self = shift;   # not really used
+    my $self = shift;
     my $barcode = shift;
     my $number  = shift or return;    # can't be zero
     return unless defined $barcode; # might be 0 or 000 or 000000

C4/SIP/ILS/Patron.pm

@@ -23,13 +23,7 @@ use C4::Reserves;
 use C4::Branch qw(GetBranchName);
 use Digest::MD5 qw(md5_base64);
 
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
-
-BEGIN {
-    $VERSION = 3.07.00.049;
-	@ISA = qw(Exporter);
-	@EXPORT_OK = qw(invalid_patron);
-}
+our $VERSION = 3.07.00.049;
 
 our $kp;	# koha patron
 
@@ -253,7 +247,7 @@ sub drop_hold {
 # from the SIP request.  Note those incoming values are 1-indexed, not 0-indexed.
 #
 sub x_items {
-    my $self      = shift or return;
+    my $self      = shift;
     my $array_var = shift or return;
     my ($start, $end) = @_;
 	$self->{$array_var} or return [];
@@ -268,28 +262,28 @@ sub x_items {
 # List of outstanding holds placed
 #
 sub hold_items {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('hold_items', @_);
 }
 
 sub overdue_items {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('overdue_items', @_);
 }
 sub charged_items {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('items', @_);
 }
 sub fine_items {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('fine_items', @_);
 }
 sub recall_items {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('recall_items', @_);
 }
 sub unavail_holds {
-    my $self = shift or return;
+    my $self = shift;
     return $self->x_items('unavail_holds', @_);
 }
 
@@ -321,16 +315,16 @@ sub inet_privileges {
 }
 
 sub fee_limit {
-    # my $self = shift;
+    my $self = shift;
     return C4::Context->preference("noissuescharge") || 5;
 }
 
 sub excessive_fees {
-    my $self = shift or return;
+    my $self = shift;
     return ($self->fee_amount and $self->fee_amount > $self->fee_limit);
 }
 sub excessive_fines {
-    my $self = shift or return;
+    my $self = shift;
     return $self->excessive_fees;   # excessive_fines is the same thing as excessive_fees for Koha
 }
     
@@ -346,10 +340,12 @@ sub library_name {
 #
 
 sub invalid_patron {
+    my $self = shift;
     return "Please contact library staff";
 }
 
 sub charge_denied {
+    my $self = shift;
     return "Please contact library staff";
 }
 

C4/SIP/ILS/Transaction/Checkin.pm

@@ -17,7 +17,7 @@ use C4::Reserves qw( ModReserveAffect );
 use C4::Items qw( ModItemTransfer );
 use C4::Debug;
 
-our @ISA = qw(ILS::Transaction);
+use parent qw(ILS::Transaction);
 
 my %fields = (
     magnetic => 0,

C4/SIP/ILS/Transaction/Checkout.pm

@@ -20,13 +20,11 @@ use C4::Circulation;
 use C4::Members;
 use C4::Reserves qw(ModReserveFill);
 use C4::Debug;
+use parent qw(ILS::Transaction);
 
-use vars qw($VERSION @ISA $debug);
+our $debug;
 
-BEGIN {
-    $VERSION = 3.07.00.049;
-	@ISA = qw(ILS::Transaction);
-}
+our $VERSION = 3.07.00.049;
 
 # Most fields are handled by the Transaction superclass
 my %fields = (

C4/SIP/ILS/Transaction/FeePayment.pm

@@ -22,9 +22,8 @@ use strict;
 
 use C4::Accounts qw(recordpayment);
 use ILS;
-use base qw(ILS::Transaction);
+use parent qw(ILS::Transaction);
 
-use vars qw($VERSION @ISA $debug);
 
 our $debug   = 0;
 our $VERSION = 3.07.00.049;

C4/SIP/ILS/Transaction/Hold.pm

@@ -12,13 +12,10 @@ use ILS::Transaction;
 use C4::Reserves;	# AddReserve
 use C4::Members;	# GetMember
 use C4::Biblio;		# GetBiblioFromItemNumber GetBiblioItemByBiblioNumber
+use parent qw(ILS::Transaction);
 
-use vars qw($VERSION @ISA);
 
-BEGIN {
-    $VERSION = 3.07.00.049;
-	    @ISA = qw(ILS::Transaction);
-}
+our $VERSION = 3.07.00.049;
 
 my %fields = (
 	expiration_date => 0,

C4/SIP/ILS/Transaction/Renew.pm

@@ -12,7 +12,7 @@ use ILS;
 use C4::Circulation;
 use C4::Members;
 
-use base qw(ILS::Transaction);
+use parent qw(ILS::Transaction);
 
 my %fields = (
     renewal_ok => 0,

C4/SIP/ILS/Transaction/RenewAll.pm

@@ -12,7 +12,7 @@ use ILS::Item;
 
 use C4::Members qw( GetMember );
 
-use base qw(ILS::Transaction::Renew);
+use parent qw(ILS::Transaction::Renew);
 
 my %fields = (
     renewed   => [],