Attachment 20157 Details for Bug 10681 – Bug 10681: Inappropriate uses of finish() in C4::Circulation.pm

[patch] Bug 10681: Inappropriate uses of finish() in C4::Circulation.pm

Bug-10681-Inappropriate-uses-of-finish-in-C4Circul.patch (text/plain), 5.76 KB, created by Chris Cormack on 2013-08-08 05:40:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Cormack

Created: 2013-08-08 05:40:20 UTC

Size: 5.76 KB

patch

obsolete

>From 679065e36eb596e156e0473de7c22292b5a3dc23 Mon Sep 17 00:00:00 2001
>From: Kenza Zaki <kenza.zaki@biblibre.com>
>Date: Mon, 5 Aug 2013 11:28:03 +0200
>Subject: [PATCH] Bug 10681: Inappropriate uses of finish() in
> C4::Circulation.pm
>
>This patch gets rid of finish().
>
>From the man page
>
>finish()
>Indicate that no more data will be fetched from this statement handle
>before it is either executed again or destroyed.
>You almost certainly do not need to call this method.
>
>Adding calls to "finish" after loop that fetches all rows is a common
>mistake, don't do it, it can mask genuine problems like uncaught fetch errors.
>
>Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
>---
> C4/Circulation.pm | 19 -------------------
> 1 file changed, 19 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index ca66931..310cefa 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -858,7 +858,6 @@ sub CanBookBeIssued {
>             my $sth = $dbh->prepare("SELECT notforloan FROM itemtypes WHERE itemtype = ?");
>             $sth->execute($item->{'itemtype'});
>             my $notforloan=$sth->fetchrow_hashref();
>-            $sth->finish();
>             if ($notforloan->{'notforloan'}) {
>                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
>                     $issuingimpossible{NOT_FOR_LOAN} = 1;
>@@ -2202,7 +2201,6 @@ sub _FixAccountForLostAndReturned {
>                 (?,?,?,?)");
>             $usth->execute($data->{'borrowernumber'},$accdata->{'accountno'},$nextaccntno,$newamtos);
>         }
>-        $msth->finish;  # $msth might actually have data left
>     }
>     $amountleft *= -1 if ($amountleft > 0);
>     my $desc = "Item Returned " . $item_id;
>@@ -2434,7 +2432,6 @@ END_SQL
>     my $sth = $dbh->prepare( $statement );
>     $sth->execute( @bind_parameters );
>     my $upcoming_dues = $sth->fetchall_arrayref({});
>-    $sth->finish;
> 
>     return $upcoming_dues;
> }
>@@ -2535,7 +2532,6 @@ sub AddRenewal {
>       );
>     $sth->execute( $borrowernumber, $itemnumber );
>     my $issuedata = $sth->fetchrow_hashref;
>-    $sth->finish;
>     if(defined $datedue && ref $datedue ne 'DateTime' ) {
>         carp 'Invalid date passed to AddRenewal.';
>         return;
>@@ -2563,7 +2559,6 @@ sub AddRenewal {
>     );
> 
>     $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
>-    $sth->finish;
> 
>     # Update the renewal count on the item, and tell zebra to reindex
>     $renews = $biblio->{'renewals'} + 1;
>@@ -2635,7 +2630,6 @@ sub GetRenewCount {
>     $sth->execute( $bornum, $itemno );
>     my $data = $sth->fetchrow_hashref;
>     $renewcount = $data->{'renewals'} if $data->{'renewals'};
>-    $sth->finish;
>     # $item and $borrower should be calculated
>     my $branchcode = _GetCircControlBranch($item, $borrower);
>     
>@@ -2704,7 +2698,6 @@ sub GetIssuingCharges {
>         }
>     }
> 
>-    $sth->finish; # we havent _explicitly_ fetched all rows
>     return ( $charge, $item_type );
> }
> 
>@@ -2766,7 +2759,6 @@ sub AddIssuingCharge {
>     ";
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $borrowernumber, $itemnumber, $nextaccntno, $charge, $charge, $manager_id );
>-    $sth->finish;
> }
> 
> =head2 GetTransfers
>@@ -2791,7 +2783,6 @@ sub GetTransfers {
>     my $sth = $dbh->prepare($query);
>     $sth->execute($itemnumber);
>     my @row = $sth->fetchrow_array();
>-    $sth->finish;
>     return @row;
> }
> 
>@@ -2821,7 +2812,6 @@ sub GetTransfersFromTo {
>     while ( my $data = $sth->fetchrow_hashref ) {
>         push @gettransfers, $data;
>     }
>-    $sth->finish;
>     return (@gettransfers);
> }
> 
>@@ -2840,7 +2830,6 @@ sub DeleteTransfer {
>          AND datearrived IS NULL "
>     );
>     $sth->execute($itemnumber);
>-    $sth->finish;
> }
> 
> =head2 AnonymiseIssueHistory
>@@ -2985,7 +2974,6 @@ sub updateWrongTransfer {
> 			"update branchtransfers set datearrived = now(),tobranch=?,comments='wrongtransfer' where itemnumber= ? AND datearrived IS NULL"
>           	);
>         	$sth->execute($FromLibrary,$itemNumber);
>-        	$sth->finish;
> 
> # second step create a new line of branchtransfer to the right location .
> 	ModItemTransfer($itemNumber, $FromLibrary, $waitingAtLibrary);
>@@ -3129,7 +3117,6 @@ my $query = qq|SELECT count(*)
> my $sth = $dbh->prepare($query);
> $sth->execute($branchcode,$week_day);
> my $result=$sth->fetchrow;
>-$sth->finish;
> return $result;
> }
> 
>@@ -3161,7 +3148,6 @@ my $query=qq|SELECT count(*)
> my $sth = $dbh->prepare($query);
> $sth->execute($years,$month,$day,$branchcode);
> my $countspecial=$sth->fetchrow ;
>-$sth->finish;
> return $countspecial;
> }
> 
>@@ -3190,7 +3176,6 @@ my $query=qq|SELECT count(*)
> my $sth = $dbh->prepare($query);
> $sth->execute($month,$day,$branchcode);
> my $countspecial=$sth->fetchrow ;
>-$sth->finish;
> return $countspecial;
> }
> 
>@@ -3206,7 +3191,6 @@ my $query=qq|SELECT count(*)
> my $sth = $dbh->prepare($query);
> $sth->execute($barcode);
> my $exist=$sth->fetchrow ;
>-$sth->finish;
> return $exist;
> }
> 
>@@ -3297,7 +3281,6 @@ sub LostItem{
>                            WHERE issues.itemnumber=?");
>     $sth->execute($itemnumber);
>     my $issues=$sth->fetchrow_hashref();
>-    $sth->finish;
> 
>     # if a borrower lost the item, add a replacement cost to the their record
>     if ( my $borrowernumber = $issues->{borrowernumber} ){
>@@ -3316,7 +3299,6 @@ sub GetOfflineOperations {
>     my $sth = $dbh->prepare("SELECT * FROM pending_offline_operations WHERE branchcode=? ORDER BY timestamp");
>     $sth->execute(C4::Context->userenv->{'branch'});
>     my $results = $sth->fetchall_arrayref({});
>-    $sth->finish;
>     return $results;
> }
> 
>@@ -3325,7 +3307,6 @@ sub GetOfflineOperation {
>     my $sth = $dbh->prepare("SELECT * FROM pending_offline_operations WHERE operationid=?");
>     $sth->execute( shift );
>     my $result = $sth->fetchrow_hashref;
>-    $sth->finish;
>     return $result;
> }
> 
>-- 
>1.8.3.1

Actions: View | Diff | Splinter Review

Attachments on bug 10681: 20107 | 20157 | 20637