From 4b7a1bdce4ebf76fd10d19704e2e5687367201bb Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Tue, 27 Aug 2013 16:02:11 +0100
Subject: [PATCH] Bug 10789 Remove unnecessary calls to finish in
 C4::Acquisitions

C4::Acquisitions contained a number of unnecessary calls to
finish. Removed these and the associated variables introduced to
cache query results between fetch and the return

Where finish was the end of the routine I have added an
explicit return to document that no data is returned.

A number of places made query calls and fetched a single
row. Such a case could require an explicit finish.
These assume that they are looking up with a uniquw key.
To remove assumptions and isolate the code from future changes
I've switched these to fetching all and returning the
first row. I have commented these cases.

For fuller explanation see perldoc DBI
---
 C4/Acquisition.pm | 141 ++++++++++++++++++++++--------------------------------
 1 file changed, 57 insertions(+), 84 deletions(-)

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 74aae85..338f0bd 100644
--- a/C4/Acquisition.pm
+++ b/C4/Acquisition.pm
@@ -428,7 +428,7 @@ sub DelBasket {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute($basketno);
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -468,7 +468,7 @@ sub ModBasket {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute(@params);
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -517,9 +517,8 @@ sub ModBasketHeader {
         my $query2 ="UPDATE aqbasket SET contractnumber=? WHERE basketno=?";
         my $sth2 = $dbh->prepare($query2);
         $sth2->execute($contractnumber,$basketno);
-        $sth2->finish;
     }
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -562,9 +561,7 @@ sub GetBasketsByBookseller {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute($booksellerid);
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return $results
+    return $sth->fetchall_arrayref({});
 }
 
 =head3 GetBasketsInfosByBookseller
@@ -632,9 +629,7 @@ sub GetBasketsByBasketgroup {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute($basketgroupid);
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return $results
+    return $sth->fetchall_arrayref({});
 }
 
 #------------------------------------------------------------#
@@ -746,10 +741,9 @@ sub ModBasketgroup {
         $sth = $dbh->prepare("UPDATE aqbasket SET basketgroupid=? WHERE basketno=?");
         foreach my $basketno (@{$basketgroupinfo->{'basketlist'}}) {
             $sth->execute($basketgroupinfo->{'id'}, $basketno);
-            $sth->finish;
         }
     }
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -775,7 +769,7 @@ sub DelBasketgroup {
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute($basketgroupid);
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -794,13 +788,13 @@ Returns a reference to the hash containing all infermation about the basketgroup
 sub GetBasketgroup {
     my $basketgroupid = shift;
     die "basketgroup id is required to edit a basketgroup" unless $basketgroupid;
-    my $query = "SELECT * FROM aqbasketgroups WHERE id=?";
     my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare($query);
-    $sth->execute($basketgroupid);
-    my $result = $sth->fetchrow_hashref;
-    $sth->finish;
-    return $result
+    my $result_set = $dbh->selectall_arrayref(
+        'SELECT * FROM aqbasketgroups WHERE id=?',
+        { Slice => {} },
+        $basketgroupid
+    );
+    return $result_set->[0];    # id is unique
 }
 
 #------------------------------------------------------------#
@@ -908,11 +902,7 @@ sub GetPendingOrders {
     }
     $strsth .= " group by aqbasket.basketno" if $grouped;
     $strsth .= " order by aqbasket.basketno";
-    my $sth = $dbh->prepare($strsth);
-    $sth->execute( @query_params );
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return $results;
+    return $dbh->selectall_arrayref( $strsth, { Slice => {} }, @query_params );
 }
 
 #------------------------------------------------------------#
@@ -950,11 +940,9 @@ sub GetOrders {
 
     $orderby = "biblioitems.publishercode,biblio.title" unless $orderby;
     $query .= " ORDER BY $orderby";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($basketno);
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return @$results;
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $basketno );
+    return @{$result_set};
 }
 
 #------------------------------------------------------------#
@@ -985,11 +973,9 @@ sub GetOrdersByBiblionumber {
             LEFT JOIN biblioitems      ON biblioitems.biblionumber =biblio.biblionumber
         WHERE   aqorders.biblionumber=?
     ";
-    my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber);
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return @$results;
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $biblionumber );
+    return @{$result_set};
 }
 
 #------------------------------------------------------------#
@@ -1016,11 +1002,11 @@ sub GetOrder {
         WHERE aqorders.ordernumber=?
 
     ";
-    my $sth= $dbh->prepare($query);
-    $sth->execute($ordernumber);
-    my $data = $sth->fetchrow_hashref;
-    $sth->finish;
-    return $data;
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $ordernumber );
+
+    # result_set assumed to contain 1 match
+    return $result_set->[0];
 }
 
 =head3 GetLastOrderNotReceivedFromSubscriptionid
@@ -1042,10 +1028,11 @@ sub GetLastOrderNotReceivedFromSubscriptionid {
             AND aqorders.datereceived IS NULL
         LIMIT 1
     |;
-    my $sth = $dbh->prepare( $query );
-    $sth->execute( $subscriptionid );
-    my $order = $sth->fetchrow_hashref;
-    return $order;
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $subscriptionid );
+
+    # result_set assumed to contain 1 match
+    return $result_set->[0];
 }
 
 =head3 GetLastOrderReceivedFromSubscriptionid
@@ -1076,10 +1063,11 @@ sub GetLastOrderReceivedFromSubscriptionid {
         ORDER BY ordernumber DESC
         LIMIT 1
     |;
-    my $sth = $dbh->prepare( $query );
-    $sth->execute( $subscriptionid, $subscriptionid );
-    my $order = $sth->fetchrow_hashref;
-    return $order;
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $subscriptionid );
+
+    # result_set assumed to contain 1 match
+    return $result_set->[0];
 
 }
 
@@ -1216,11 +1204,10 @@ sub ModOrder {
     }
 
     $query .= "timestamp=NOW()  WHERE  ordernumber=?";
-#   push(@params, $specorderinfo{'ordernumber'});
     push(@params, $orderinfo->{'ordernumber'} );
     $sth = $dbh->prepare($query);
     $sth->execute(@params);
-    $sth->finish;
+    return;
 }
 
 #------------------------------------------------------------#
@@ -1322,13 +1309,13 @@ sub ModReceiveOrder {
                         );
     }
 
-    my $sth=$dbh->prepare("
-        SELECT * FROM   aqorders
-        WHERE           biblionumber=? AND aqorders.ordernumber=?");
+    my $result_set = $dbh->selectall_arrayref(
+q{SELECT * FROM aqorders WHERE biblionumber=? AND aqorders.ordernumber=?},
+        { Slice => {} }, $biblionumber, $ordernumber
+    );
 
-    $sth->execute($biblionumber,$ordernumber);
-    my $order = $sth->fetchrow_hashref();
-    $sth->finish();
+    # we assume we have a unique order
+    my $order = $result_set->[0];
 
     my $new_ordernumber = $ordernumber;
     if ( $order->{quantity} > $quantrec ) {
@@ -1336,7 +1323,7 @@ sub ModReceiveOrder {
         # without received items (the quantity is decreased),
         # the second part is a new order line with quantity=quantityrec
         # (entirely received)
-        $sth=$dbh->prepare("
+        my $sth=$dbh->prepare("
             UPDATE aqorders
             SET quantity = ?
             WHERE ordernumber = ?
@@ -1344,8 +1331,6 @@ sub ModReceiveOrder {
 
         $sth->execute($order->{quantity} - $quantrec, $ordernumber);
 
-        $sth->finish;
-
         delete $order->{'ordernumber'};
         $order->{'quantity'} = $quantrec;
         $order->{'quantityreceived'} = $quantrec;
@@ -1364,12 +1349,11 @@ sub ModReceiveOrder {
             }
         }
     } else {
-        $sth=$dbh->prepare("update aqorders
+        my $sth=$dbh->prepare("update aqorders
                             set quantityreceived=?,datereceived=?,invoiceid=?,
                                 unitprice=?,rrp=?,ecost=?
                             where biblionumber=? and ordernumber=?");
         $sth->execute($quantrec,$datereceived,$invoiceid,$cost,$rrp,$ecost,$biblionumber,$ordernumber);
-        $sth->finish;
     }
     return ($datereceived, $new_ordernumber);
 }
@@ -1561,9 +1545,7 @@ sub SearchOrder {
 
     my $sth = $dbh->prepare($query);
     $sth->execute(@args);
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return $results;
+    return $sth->fetchall_arrayref({});
 }
 
 #------------------------------------------------------------#
@@ -1588,12 +1570,11 @@ sub DelOrder {
     ";
     my $sth = $dbh->prepare($query);
     $sth->execute( $bibnum, $ordernumber );
-    $sth->finish;
     my @itemnumbers = GetItemnumbersFromOrder( $ordernumber );
     foreach my $itemnumber (@itemnumbers){
     	C4::Items::DelItem( $dbh, $bibnum, $itemnumber );
     }
-    
+    return;
 }
 
 =head2 FUNCTIONS ABOUT PARCELS
@@ -1660,16 +1641,13 @@ sub GetParcel {
         }
     }
     $strsth .= " ORDER BY aqbasket.basketno";
-    # ## parcelinformation : $strsth
-    my $sth = $dbh->prepare($strsth);
-    $sth->execute( @query_params );
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
-    }
-    # ## countparcelbiblio: scalar(@results)
-    $sth->finish;
 
-    return @results;
+    my $result_set = $dbh->selectall_arrayref(
+        $strsth,
+        { Slice => {} },
+        @query_params);
+
+    return @{$result_set};
 }
 
 #------------------------------------------------------------#
@@ -1757,8 +1735,7 @@ sub GetParcels {
 
     $sth->execute( @query_params );
     my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return @$results;
+    return @{$results};
 }
 
 #------------------------------------------------------------#
@@ -2119,14 +2096,10 @@ sub GetContracts {
             WHERE booksellerid=?
                 AND contractenddate >= CURDATE( )";
     }
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $booksellerid );
-    my @results;
-    while (my $data = $sth->fetchrow_hashref ) {
-        push(@results, $data);
-    }
-    $sth->finish;
-    return @results;
+
+    my $result_set =
+      $dbh->selectall_arrayref( $query, { Slice => {} }, $booksellerid );
+    return @{$result_set};
 }
 
 #------------------------------------------------------------#
-- 
1.8.4.rc2.15.g96cb27a