From f16e406063489bcd6fed80d61133d9c351c2a441 Mon Sep 17 00:00:00 2001 From: Juerd Waalboer Date: Wed, 28 Aug 2024 06:41:02 +0200 Subject: [PATCH] Revert "Handle huge numbers better" This reverts commit ef0039bc335e4e8c82e4f4be2b4af84cc067a052. Abysmal performance: revbank2beancount went from 0.7 to 11 seconds for revspace's 2024 .revbank.log to date. --- lib/RevBank/Amount.pm | 29 ++++++++++++++----------- lib/RevBank/Global.pm | 50 ++++++++++++++++++------------------------- plugins/adduser | 5 ++--- revbank | 3 ++- t/amount.t | 8 ------- 5 files changed, 41 insertions(+), 54 deletions(-) diff --git a/lib/RevBank/Amount.pm b/lib/RevBank/Amount.pm index 5f3f351..98c97df 100644 --- a/lib/RevBank/Amount.pm +++ b/lib/RevBank/Amount.pm @@ -8,7 +8,6 @@ use experimental 'signatures'; # stable since v5.36 use Carp qw(carp croak); use Scalar::Util; use POSIX qw(lround); -use Math::BigInt; our $C = __PACKAGE__; @@ -42,12 +41,12 @@ use overload ( }, "*" => sub ($a, $b, $swap) { $b = $b->_float_warn if ref $b; - $C->new($$a->as_float * ($b->can('float') ? $b->float : $b->can('as_float') ? $b->as_float : $b)); + $C->new($$a * $b); }, "/" => sub ($a, $b, $swap) { carp "Using floating-point arithmetic for $a/$b (use \$amount->float to suppress warning)"; $b = $b->float if ref $b; - $C->new($$a->as_float / $b); + $C->new($$a / $b); }, "<=>" => sub ($a, $b, $swap) { _coerce($a, $b); @@ -60,15 +59,19 @@ use overload ( ); sub new($class, $cents) { - my $int = Math::BigInt->new($cents->can("as_int") ? $cents->as_int : $cents); - croak "Non-integer not supported" if $int->is_nan; - croak "Infinite number not supported" if $int->is_inf; - + my $int = int sprintf "%d", $cents; + #carp "$cents rounded to $int cents" if $int != $cents; return bless \$int, $class; } sub new_from_float($class, $num) { - return $class->new((100 * Math::BigFloat->new($num))->bfround(0)->as_int); + return $class->new(lround 100 * sprintf "%.02f", $num); + + # First, round the float with sprintf for bankers rounding, then + # multiply to get number of cents. However, 4.56 as a float is + # 4.55999... which with int would get truncated to 455, so lround is needed + # to get 456. + # Note: _l_round, because normal round gives the int *AS A FLOAT*; sigh. } sub parse_string($class, $str) { @@ -85,7 +88,7 @@ sub parse_string($class, $str) { $cents *= 10 if length($cents) == 1; # 4.2 -> 4.20 return $class->new( - ($neg ? -1 : 1) * (100 * Math::BigInt->new($int) + $cents) + ($neg ? -1 : 1) * ($int * 100 + $cents) ); } @@ -94,7 +97,7 @@ sub cents($self) { } sub float($self) { - return $$self->as_float / 100; + return $$self / 100; } sub _float_warn($self) { @@ -104,9 +107,9 @@ sub _float_warn($self) { sub string($self, $plus = "") { return sprintf( - "%s%s.%02d", + "%s%d.%02d", $$self < 0 ? "-" : $plus, - abs($$self) / 100, # %s for bigint + abs($$self) / 100, abs($$self) % 100, ); } @@ -116,7 +119,7 @@ sub string_flipped($self, $sep = " ") { "%s%s%d.%02d", $$self > 0 ? "+" : "", $$self > 0 ? $sep : "", - abs($$self) / 100, # %s for bigint + abs($$self) / 100, abs($$self) % 100, ); } diff --git a/lib/RevBank/Global.pm b/lib/RevBank/Global.pm index a55a5d2..c5240e6 100644 --- a/lib/RevBank/Global.pm +++ b/lib/RevBank/Global.pm @@ -14,32 +14,6 @@ use RevBank::FileIO; sub reason($self) { return $$self; } } -sub _parse_any_amount($amount) { - defined $amount or return undef; - length $amount or return undef; - - my @split = grep /\S/, split /([+-])/, $amount; - - my $posneg = 1; - $amount = RevBank::Amount->new(0); - for my $token (@split) { - if ($token eq '-') { - $posneg = $posneg == -1 ? 1 : -1; - } elsif ($token eq '+') { - $posneg ||= 1; - } else { - $posneg or return undef; # two terms in a row - my $term = RevBank::Amount->parse_string($token) // return undef; - $amount += $posneg * $term; - $posneg = 0; - } - } - $posneg and return undef; # last token must be term - - return $amount; - -} - sub import { require RevBank::Plugins; require RevBank::Users; @@ -57,9 +31,27 @@ sub import { *{"$caller\::append"} = \&RevBank::FileIO::append; *{"$caller\::with_lock"} = \&RevBank::FileIO::with_lock; *{"$caller\::parse_user"} = \&RevBank::Users::parse_user; - *{"$caller\::parse_any_amount"} = \&_parse_any_amount; - *{"$caller\::parse_amount" } = sub ($amount) { - $amount = _parse_any_amount($amount) // return undef; + *{"$caller\::parse_amount"} = sub ($amount) { + defined $amount or return undef; + length $amount or return undef; + + my @split = grep /\S/, split /([+-])/, $amount; + + my $posneg = 1; + $amount = RevBank::Amount->new(0); + for my $token (@split) { + if ($token eq '-') { + $posneg = $posneg == -1 ? 1 : -1; + } elsif ($token eq '+') { + $posneg ||= 1; + } else { + $posneg or return undef; # two terms in a row + my $term = RevBank::Amount->parse_string($token) // return undef; + $amount += $posneg * $term; + $posneg = 0; + } + } + $posneg and return undef; # last token must be term if ($amount->cents < 0) { die RevBank::Exception::RejectInput->new( diff --git a/plugins/adduser b/plugins/adduser index a6e9353..3c9f83d 100644 --- a/plugins/adduser +++ b/plugins/adduser @@ -21,9 +21,8 @@ sub username($self, $cart, $name, @) { return REJECT, "Sorry, - + / ^ * are not allowed as the first character." if $name =~ /^[-+*\/\^]/; - my $num; - return REJECT, "Sorry, that evaluates to the amount $num and can't be an account name." - if defined($num = parse_any_amount($name)); + return REJECT, "Sorry, that's too numeric to be a user name." + if defined RevBank::Amount->parse_string($name); return REJECT, "That name is not available." if defined parse_user($name, 1); diff --git a/revbank b/revbank index af9fa7f..bf66a16 100755 --- a/revbank +++ b/revbank @@ -17,7 +17,8 @@ use RevBank::Messages; use RevBank::Cart; use RevBank::Prompt; -our $VERSION = "6.2.3"; +our $VERSION = "6.2.4"; + our %HELP1 = ( "abort" => "Abort the current transaction", ); diff --git a/t/amount.t b/t/amount.t index d1024cf..9318e8e 100644 --- a/t/amount.t +++ b/t/amount.t @@ -40,12 +40,6 @@ cmp_ok(RevBank::Amount->parse_string("4 ")->cents, '==', 400); cmp_ok(RevBank::Amount->new_from_float(.425)->cents, '==', 42); - -my $big = "9" x 123; -cmp_ok(RevBank::Amount->new($big)->cents, 'eq', $big); -cmp_ok(RevBank::Amount->parse_string($big)->cents, 'eq', $big . "00"); -cmp_ok(RevBank::Amount->new_from_float($big)->cents, 'eq', $big . "00"); - # comparisons ok($a); ok(!RevBank::Amount->new(0)); @@ -142,8 +136,6 @@ like(warning { rand $a }, qr/float/); # Invalid amounts -throws_ok(sub { RevBank::Amount->new(1.1) }, qr/Non-integer/); -throws_ok(sub { RevBank::Amount->new("+Inf") }, qr/Infinite/); is(RevBank::Amount->parse_string("0.000"), undef); is(RevBank::Amount->parse_string("0.042"), undef); is(RevBank::Amount->parse_string("+0.042"), undef);