From b27ed31aa31fb9098fcee568564a7a92c884032e Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 18 Mar 2013 14:37:18 +0000 Subject: [PATCH] Bug 9823: Refactor return from GetReservesFromBiblionumber The return from GetReservesFromBiblionumber contains an unnecessary extra variable. In scalar context an array returns its element count. Maintaing a separate count can lead to unforeseen bugs and imposes ugly constructions on the subroutine's users. Remove the useless count variable from the return --- C4/Biblio.pm | 2 +- C4/ILSDI/Services.pm | 4 ++-- C4/Reserves.pm | 4 ++-- C4/SIP/ILS/Item.pm | 4 ++-- catalogue/ISBDdetail.pl | 3 ++- catalogue/MARCdetail.pl | 3 ++- catalogue/detail.pl | 3 ++- catalogue/imageviewer.pl | 3 ++- catalogue/labeledMARCdetail.pl | 3 ++- catalogue/moredetail.pl | 3 ++- opac/opac-detail.pl | 4 ++-- opac/opac-reserve.pl | 4 ++-- reserve/request.pl | 5 +++-- serials/routing-preview.pl | 5 +++-- 14 files changed, 29 insertions(+), 21 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 16134d4..77f0243 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -437,7 +437,7 @@ sub DelBiblio { # We delete any existing holds require C4::Reserves; - my ($count, $reserves) = C4::Reserves::GetReservesFromBiblionumber($biblionumber); + my $reserves = C4::Reserves::GetReservesFromBiblionumber($biblionumber); foreach my $res ( @$reserves ) { C4::Reserves::CancelReserve( $res->{'biblionumber'}, $res->{'itemnumber'}, $res->{'borrowernumber'} ); } diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 68cbdb3..30d17f0 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -208,7 +208,7 @@ sub GetRecords { # Get most of the needed data my $biblioitemnumber = $biblioitem->{'biblioitemnumber'}; - my @reserves = GetReservesFromBiblionumber( $biblionumber, undef, undef ); + my $reserves = GetReservesFromBiblionumber( $biblionumber, undef, undef ); my $issues = GetBiblioIssues($biblionumber); my $items = GetItemsByBiblioitemnumber($biblioitemnumber); @@ -225,7 +225,7 @@ sub GetRecords { # Hashref building... $biblioitem->{'items'}->{'item'} = $items; - $biblioitem->{'reserves'}->{'reserve'} = $reserves[1]; + $biblioitem->{'reserves'}->{'reserve'} = $reserves; $biblioitem->{'issues'}->{'issue'} = $issues; push @records, $biblioitem; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index ba12cf5..98329ae 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -254,7 +254,7 @@ of the reserves and an arrayref pointing to the reserves for C<$biblionumber>. =cut sub GetReservesFromBiblionumber { - my ($biblionumber) = shift or return (0, []); + my ($biblionumber) = shift or return []; my ($all_dates) = shift; my $dbh = C4::Context->dbh; @@ -323,7 +323,7 @@ sub GetReservesFromBiblionumber { } push @results, $data; } - return ( $#results + 1, \@results ); + return \@results; } =head2 GetReservesFromItemnumber diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm index ba85cd7..a590a96 100644 --- a/C4/SIP/ILS/Item.pm +++ b/C4/SIP/ILS/Item.pm @@ -95,8 +95,8 @@ sub new { } my $borrower = GetMember(borrowernumber=>$issue->{'borrowernumber'}); $item->{patron} = $borrower->{'cardnumber'}; - my ($whatever, $arrayref) = GetReservesFromBiblionumber($item->{biblionumber}); - $item->{hold_queue} = [ sort priority_sort @$arrayref ]; + my $reserves = GetReservesFromBiblionumber($item->{biblionumber}); + $item->{hold_queue} = [ sort priority_sort @$reserves ]; $item->{hold_shelf} = [( grep { defined $_->{found} and $_->{found} eq 'W' } @{$item->{hold_queue}} )]; $item->{pending_queue} = [( grep {(! defined $_->{found}) or $_->{found} ne 'W' } @{$item->{hold_queue}} )]; $self = $item; diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl index f094160..9f8de42 100755 --- a/catalogue/ISBDdetail.pl +++ b/catalogue/ISBDdetail.pl @@ -112,7 +112,8 @@ $template->param ( ); -my ( $holdcount, $holds ) = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holds = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl index d8079bd..84e89a8 100755 --- a/catalogue/MARCdetail.pl +++ b/catalogue/MARCdetail.pl @@ -335,7 +335,8 @@ $template->param ( C4::Search::enabled_staff_search_views, ); -my ( $holdcount, $holds ) = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holds = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/detail.pl b/catalogue/detail.pl index ec6f6eb..34a5d42 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -395,7 +395,8 @@ if (C4::Context->preference('TagsEnabled') and $tag_quantity = C4::Context->pref 'sort'=>'-weight', limit=>$tag_quantity})); } -my ( $holdcount, $holds ) = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holds = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar ( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/imageviewer.pl b/catalogue/imageviewer.pl index c96e5af..118aa8a 100755 --- a/catalogue/imageviewer.pl +++ b/catalogue/imageviewer.pl @@ -78,7 +78,8 @@ $template->{VARS}->{'norequests'} = $norequests; $template->param(C4::Search::enabled_staff_search_views); $template->{VARS}->{'biblio'} = $biblio; -my ( $holdcount, $holds ) = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holds = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/labeledMARCdetail.pl b/catalogue/labeledMARCdetail.pl index 9d2a14e..a34aab7 100755 --- a/catalogue/labeledMARCdetail.pl +++ b/catalogue/labeledMARCdetail.pl @@ -136,7 +136,8 @@ $template->param ( C4::Search::enabled_staff_search_views, ); -my ( $holdcount, $holds ) = C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holds= C4::Reserves::GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl index 55e8273..c16486a 100755 --- a/catalogue/moredetail.pl +++ b/catalogue/moredetail.pl @@ -216,7 +216,8 @@ $template->param( $template->param(ONLY_ONE => 1) if ( $itemnumber && $showncount != @items ); -my ( $holdcount, $holds ) = GetReservesFromBiblionumber($biblionumber,1); +my $holds = GetReservesFromBiblionumber($biblionumber,1); +my $holdcount = scalar( @$holds ); $template->param( holdcount => $holdcount, holds => $holds ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 068122e..1d851fa 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -496,8 +496,8 @@ for ( C4::Context->preference("OPACShowHoldQueueDetails") ) { } my $has_hold; if ( $show_holds_count || $show_priority) { - my ($reserve_count,$reserves) = GetReservesFromBiblionumber($biblionumber); - $template->param( holds_count => $reserve_count ) if $show_holds_count; + my $reserves = GetReservesFromBiblionumber($biblionumber); + $template->param( holds_count => scalar( @$reserves ) ) if $show_holds_count; foreach (@$reserves) { $item_reserves{ $_->{itemnumber} }++ if $_->{itemnumber}; if ($show_priority && $_->{borrowernumber} == $borrowernumber) { diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 07b4f76..6617b37 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -148,8 +148,8 @@ foreach my $biblioNumber (@biblionumbers) { } # Compute the priority rank. - my ( $rank, $reserves ) = - GetReservesFromBiblionumber( $biblioNumber, 1 ); + my $reserves = GetReservesFromBiblionumber( $biblioNumber, 1 ); + my $rank = scalar( @$reserves ); $biblioData->{reservecount} = 1; # new reserve foreach my $res (@{$reserves}) { my $found = $res->{found}; diff --git a/reserve/request.pl b/reserve/request.pl index 506d5fa..a805940 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -255,7 +255,8 @@ foreach my $biblionumber (@biblionumbers) { } # get existing reserves ..... - my ( $count, $reserves ) = GetReservesFromBiblionumber($biblionumber,1); + my $reserves = GetReservesFromBiblionumber($biblionumber,1); + my $count = scalar( @$reserves ); my $totalcount = $count; my $alreadyreserved; @@ -512,7 +513,7 @@ foreach my $biblionumber (@biblionumbers) { # existingreserves building my @reserveloop; - ( $count, $reserves ) = GetReservesFromBiblionumber($biblionumber,1); + $reserves = GetReservesFromBiblionumber($biblionumber,1); foreach my $res ( sort { my $a_found = $a->{found} || ''; my $b_found = $a->{found} || ''; diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl index 437c852..70a9522 100755 --- a/serials/routing-preview.pl +++ b/serials/routing-preview.pl @@ -71,8 +71,9 @@ if($ok){ if (C4::Context->preference('RoutingListAddReserves')){ # get existing reserves ..... - my ($count,$reserves) = GetReservesFromBiblionumber($biblio); - my $totalcount = $count; + my $reserves = GetReservesFromBiblionumber($biblio); + my $count = scalar( @$reserves ); + my $totalcount = $count; foreach my $res (@$reserves) { if ($res->{'found'} eq 'W') { $count--; -- 1.8.2.6.g1c71541