[Koha-patches] [PATCH] Bug 8429: Remove unnecessary use of Exporter from SIP/ILS
Colin Campbell
colin.campbell at ptfs-europe.com
Thu Jul 12 17:46:19 CEST 2012
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 38951fc..0df9072 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 57db003..c6f34fa 100644
--- a/C4/SIP/ILS/Transaction/Renew.pm
+++ b/C4/SIP/ILS/Transaction/Renew.pm
@@ -13,7 +13,7 @@ use ILS::Transaction;
use C4::Circulation;
use C4::Members;
-our @ISA = 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 adc467a..e29d070 100644
--- a/C4/SIP/ILS/Transaction/RenewAll.pm
+++ b/C4/SIP/ILS/Transaction/RenewAll.pm
@@ -13,7 +13,7 @@ use ILS::Transaction::Renew;
use C4::Members; # GetMember
-our @ISA = qw(ILS::Transaction::Renew);
+use parent qw(ILS::Transaction::Renew);
my %fields = (
renewed => [],
--
1.7.11.1.165.g299666c
More information about the Koha-patches
mailing list