[Question] Can we improve representation of Money?

[samitnuk]

For example, I have Pay-In with the next data (screenshot from the dashboard):

When I check this Pay-In in the terminal I have:

In [3]: p = DirectDebitWebPayIn.get('xxxxxxxxxx')

In [4]: p.debited_funds
Out[4]: EUR 18043

In [5]: p.fees
Out[5]: EUR 6542

From my point of view, these values should be represented as EUR 180.43 and EUR 65.42 (as in the dashboard).

To fix this I can suggest to use self.amount / 100 instead of self.amount here:

mangopay2-python-sdk/mangopay/utils.py

Lines 62 to 63 in 2a656a8

	def __repr__(self):
	return "{} {}".format(self.currency, self.amount)

and here:

mangopay2-python-sdk/mangopay/utils.py

Lines 65 to 66 in 2a656a8

	def __str__(self):
	return force_text("{} {:,.2f}".format(self.currency, self.amount))

But of course, this "fix" will work only for the currencies with 2 digits after the decimal separator. So maybe there can be a better solution.

Also, I understand that Money in the SDK represents An amount of money in the smallest sub-division of the currency, so showing 18043 instead of 180.43 can be correct from some point of view. But we show currency near it and this can be confusing.

But I think the minimum that can be done here is to use for __str__ the same functionality as for __repr__ (means remove :,.2f. For example, when I want to log Pay-In values I have something like next in my logs:

Booking #000000001: Created Pay-In #0000000002, debited funds - EUR 18,043.00, fees - EUR 6,542.00

As you can see, the __str__ method is definitely produces an incorrect representation of the values from above.

[samitnuk]

In [3]: preauthorization.debited_funds
Out[3]: EUR 2495

In [4]: str(preauthorization.debited_funds)
Out[4]: 'EUR 2,495.00'

In [5]: vars(preauthorization.debited_funds)
Out[5]: {'amount': Decimal('2495'), 'currency': 'EUR'}

But the correct number to show is 24.95. So to show the "blocked" amount of money for a user we need somehow format this value by ourselves. E.g.

In [6]: preauthorization.debited_funds.amount / 100
Out[6]: Decimal('24.95')

or

In [7]: preauthorization.debited_funds / 100
Out[7]: EUR 24.95

[yoch]

It is not always correct to divide by 100 because not all currencies have two decimals, see ISO 4217.

In fact, there's no need to use Decimal here at all and it can get confusing as the Mangopay documentation indicates that int type is always used for MoneyCard amounts:

An amount of money in the smallest sub-division of the currency, e.g. 12.60 EUR 
would be represented as 1260 whereas 12 JPY would be represented as just 12

So the easiest option is to just use int here, and avoid the {:,.2f} format which introduces a lot of confusion.

If you want to improve the Money class, it makes sense to use Decimal internally, but the amount should be corrected accordingly to the number of decimals, like amount = Decimal(amount) / (10 **currency.decimals). But for that ISO 4217 support is needed (some packages exists).

[shanx]

In our project we are actually patching MoneyField to hide the division/multiplication as an implementation detail instead of sprinkling them all over our codebase. Is there any specific reason why customer implementations are made responsible to deal with this? In my view the mangopay SDK's have all the information they need to hide this. I think also not hiding this is a dangerous vector for client codebases to introduce bugs on their end.

[jpic]

Not reason to not encapsulate this dance inside the SDK, I mean, mango.Money(amount=Decimal('10.10'), currency='EUR') means actually 0.10€ 😂