[open-ils-commits] r16092 - trunk/Open-ILS/src/perlmods/OpenILS/Application (erickson)

[svn at svn.open-ils.org]

Author: erickson
Date: 2010-04-01 15:09:33 -0400 (Thu, 01 Apr 2010)
New Revision: 16092

Modified:
   trunk/Open-ILS/src/perlmods/OpenILS/Application/Actor.pm
Log:
Patch from Joe Atzberger:  Cleanup subs in Actor

For the first, @xact was never used.
The 3 part conditional only had two different outcomes.
There is still a problem with user_transactions reproducing a lot
of the functionality of user_transaction_history, which it already
calls to do the real query.  But it doesn't reproduce all of it.
Recommend relying on user_transaction_history as much as possible,
but that kind of work is out of scope for this review.

For the other, remove unused var and unnecessary array.
You don't need to build an array when all you do at the end is count its elements.

Signed-off-by: Joe Atzberger <atz at esilibrary.com>

Modified: trunk/Open-ILS/src/perlmods/OpenILS/Application/Actor.pm
===================================================================
--- trunk/Open-ILS/src/perlmods/OpenILS/Application/Actor.pm	2010-04-01 19:09:32 UTC (rev 16091)
+++ trunk/Open-ILS/src/perlmods/OpenILS/Application/Actor.pm	2010-04-01 19:09:33 UTC (rev 16092)
@@ -1641,32 +1641,16 @@
 		$login_session, $user_id, 'VIEW_USER_TRANSACTIONS' );
 	return $evt if $evt;
 
-	my $api = $self->api_name();
-	my $trans;
-	my @xact;
+    my $api = $self->api_name();
 
-	if(defined($type)) { @xact = (xact_type =>  $type); 
+    my ($trans) = $self->method_lookup(
+        'open-ils.actor.user.transactions.history.still_open')
+      ->run( $login_session, $user_id, $type );
 
-	} else { @xact = (); }
+    $trans = ($api =~ /have_balance/o) ?
+            [ grep { int($_->balance_owed * 100) != 0 } @$trans ] : 
+            [ grep { int($_->total_owed   * 100)  > 0 } @$trans ] ;
 
-	($trans) = $self
-		->method_lookup('open-ils.actor.user.transactions.history.still_open')
-		->run($login_session => $user_id => $type);
-
-
-	if($api =~ /have_charge/o) {
-
-		$trans = [ grep { int($_->total_owed * 100) > 0 } @$trans ];
-
-	} elsif($api =~ /have_balance/o) {
-
-		$trans = [ grep { int($_->balance_owed * 100) != 0 } @$trans ];
-	} else {
-
-		$trans = [ grep { int($_->total_owed * 100) > 0 } @$trans ];
-
-	}
-
 	if($api =~ /total/o) { 
 		my $total = 0.0;
 		for my $t (@$trans) {
@@ -1677,8 +1661,8 @@
 		return $total;
 	}
 
-	if($api =~ /count/o) { return scalar @$trans; }
-	if($api !~ /fleshed/o) { return $trans; }
+    ($api =~ /count/o  ) and return scalar @$trans;
+    ($api !~ /fleshed/o) and return $trans;
 
 	my @resp;
 	for my $t (@$trans) {
@@ -1863,17 +1847,15 @@
 
 	my $parser = DateTime::Format::ISO8601->new;
 
-	my (@out, at overdue);
+	my $overdue = 0;
 	for my $c (@$circs) {
 		my $due_dt = $parser->parse_datetime( cleanse_ISO8601( $c->due_date ) );
-		my $due = $due_dt->epoch;
-
-		if ($due < DateTime->today->epoch) {
-			push @overdue, $c;
+		if ($due_dt->epoch < DateTime->today->epoch) {
+			$overdue++;
 		}
 	}
 
-	return { total => scalar(@$circs), overdue => scalar(@overdue) };
+	return { total => scalar(@$circs), overdue => $overdue };
 }