Attachment 20654 Details for Bug 10789 – Proposed Patch

[patch] Proposed Patch

0001-Bug-10789-Remove-unnecessary-calls-to-finish-in-C4-A.patch (text/plain), 11.19 KB, created by Colin Campbell on 2013-08-27 15:26:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Campbell

Created: 2013-08-27 15:26:00 UTC

Size: 11.19 KB

patch

obsolete

>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
>

Actions: View | Diff | Splinter Review

Attachments on bug 10789: 20654 | 23349 | 25350 | 25371 | 25378 | 25379 | 25380 | 25381 | 25382