Bugzilla – Attachment 13360 Details for
Bug 8429
Unnecessary use of Exporter in SIP/ILS objects
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 8429: Remove unnecessary use of Exporter from SIP/ILS
Bug-8429-Remove-unnecessary-use-of-Exporter-from-S.patch (text/plain), 8.36 KB, created by
Kyle M Hall (khall)
on 2012-11-09 16:32:52 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 8429: Remove unnecessary use of Exporter from SIP/ILS
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2012-11-09 16:32:52 UTC
Size:
8.36 KB
patch
obsolete
>From 671074a9d3aa88a42ec2cceb92032bcb98344d35 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Fri, 6 Jul 2012 16:19:33 +0100 >Subject: [PATCH] Bug 8429: Remove unnecessary use of Exporter from SIP/ILS >Content-Type: text/plain; charset="utf-8" > >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> >--- > 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 fdd79a8..eae2383 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.2.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8429
:
10796
|
11685
|
12333
| 13360