From 7e19f5650ada7153f775298ae53193d94865824c Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Fri, 6 Jul 2012 16:19:33 +0100 Subject: [PATCH] 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 --- C4/SIP/ILS/Item.pm | 19 +++++++------------ C4/SIP/ILS/Patron.pm | 30 +++++++++++++----------------- C4/SIP/ILS/Transaction/Checkin.pm | 2 +- C4/SIP/ILS/Transaction/Checkout.pm | 8 +++----- C4/SIP/ILS/Transaction/FeePayment.pm | 3 +-- C4/SIP/ILS/Transaction/Hold.pm | 7 ++----- C4/SIP/ILS/Transaction/Renew.pm | 2 +- C4/SIP/ILS/Transaction/RenewAll.pm | 2 +- 8 files changed, 29 insertions(+), 44 deletions(-) diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm index 6bf7192..ba85cd7 100644 --- a/C4/SIP/ILS/Item.pm +++ b/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 diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index d3a9762..69389eb 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/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"; } diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm index 47bd85d..49ce926 100644 --- a/C4/SIP/ILS/Transaction/Checkin.pm +++ b/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, diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index da7a1b0..c61eca6 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/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 = ( diff --git a/C4/SIP/ILS/Transaction/FeePayment.pm b/C4/SIP/ILS/Transaction/FeePayment.pm index 19bdd04..52619f4 100644 --- a/C4/SIP/ILS/Transaction/FeePayment.pm +++ b/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; diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm index 3352c1c..a2ca13e 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/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, diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm index 1f0f567..aa300d4 100644 --- a/C4/SIP/ILS/Transaction/Renew.pm +++ b/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, diff --git a/C4/SIP/ILS/Transaction/RenewAll.pm b/C4/SIP/ILS/Transaction/RenewAll.pm index c7be96b..7cdb4bb 100644 --- a/C4/SIP/ILS/Transaction/RenewAll.pm +++ b/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 => [], -- 1.7.12.464.g83379df