manwar / Data-Money

Money/currency with formatting and overloading.
Artistic License 2.0
4 stars 3 forks source link

Mutable state failure #5

Open mrenvoize opened 3 years ago

mrenvoize commented 3 years ago
#!/usr/bin/env perl

use strict;
use warnings;

use Data::Money;

my $debt   = 90.00;  # Debts are stored as positives in the database
my $debt_money = Data::Money->new(value => $debt, code => 'GBP');
my $subtraction = $debt_money;

print "Debt: " . $debt . "\n";
print "Debt Money: " . $debt_money . "\n\n";

print "Doing money sum $debt_money -= $subtraction\n";
$debt_money -= $subtraction;
print "Money amount after:" . $debt_money . "\n";

The above script highlights a mutable state issue; In the above snippet we can see in this case that the in place subtraction £90 --= $90 should result in £0 but actually yields -£90.

I asked on #perl-help and it appears to stem from $amount_to_subtract containing the same object as $debt_money.. which is the case when $credit_money >= $debt_money.

mrenvoize commented 3 years ago

I tried using the clone method above where we set $subtraction but that loses the value internally. I'm not sure why the code is written that way, and the documentation for clone does not state this is the case (though clearly from the code it is).

Should there be a 'copy' method akin to that found in Math::BigFloat?

I'm also still wondering about my own use case, is my code simply being nieve to think that having a reference to the same object on both sides should work in this case (putting aside the idea of using switching to using clone.. I was hoping to basically outright replace our use of simple numeric variables with Data::Money objects without having to make too many changes to the existing code like requiring clones/copies taking place)

mrenvoize commented 3 years ago

We updated the behaviour of 'clone' to properly handle the internal 'value' so the above issue can be resolved cleanly by simply calling ->clone in the assignment.

I will be writing some tests to verify this and then I believe we can also close this issue. In theory, the = operator is overloaded to call clone.. but I'm not sure I was actually seeing that working in the example above.