From 4b7a1bdce4ebf76fd10d19704e2e5687367201bb Mon Sep 17 00:00:00 2001 From: Colin Campbell 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