Since you are using f strings I will presume you are using python 3.6 or later.
def __init__(self, value):
self._cents = int(round(value * 100, 0))
Good job prefixing cents with an underscore. I think it is a good choice to make the variable "private".
By leaving out the second parameter from round, or by passing None, the returned value will be rounded to an integer. You could instead write
def __init__(self, value):
self._cents = round(value * 100)
Note that round may not behave as you expect. Depending on the value given it may round the opposite way to what you'd expect.
def __add__(self, other):
return self.__class__((self._cents + other._cents) / 100)
It is good that you've accounted for subclassing. Since you've had to do some work to make a new class to return, maybe that indicates you could use an alternative constructor that takes cents directly?
Unfortunately you don't do any type checking here, so the error message a user receives is not very helpful.
a = Currency(2.345)
print(a) # 2.35
a + 2.03 # AttributeError: 'float' object has no attribute '_cents'
I would suggest type checking as you've done in mul, making sure only currency can be added to currency
def __add__(self, other):
if not isinstance(other, Currency):
raise TypeError(f"Unsupported type, cannot add {self.__class__} and {other.__class__}")
...
def __mul__(self, factor):
if type(factor) not in [int, float]:
raise ValueError('factor must be a scalar')
From this stackoverflow answer, isinstance is preferred over type. Also from the same answer, duck typing is preferred over isinstance. Since we don't want to have to update this list every time we find a new type that is acceptable to multiply by, lets just try it and see if it works. Multiplying a currency will then just work unless there is a reason it shouldn't. One exception might be multiplying currency by currency. Should that be allowed?
def __mul__(self, factor):
try:
return self.__class__(factor * self._cents / 100)
except TypeError:
raise TypeError(f"...")
This isn't perfect, as we may lose useful information about what doesn't work (Did the constructor fail? Did the multiplication fail? etc) but it is good enough.
# Example of currency just working with fractions
a = Currency(8.08)
a * fraction.Fraction(1, 4) # 2.02
def __truediv__(self, divisor):
if type(divisor) not in [int, float]:
raise ValueError('divisor must be a scalar')
if divisor != 0:
return self.__class__(self._cents / divisor / 100)
else:
raise ValueError('Cannot divide by zero')
Since an exception is raised if you try to divide a currency by 0, I don't see an advantage of making it a ValueError over a ZeroDivisionError. It wold actually remove the error handling since it doesn't add much, and let the user catch it if they want to.
If there is a usecase for it, it may be worth defining div too. It could be as simple as
__div__ = __truediv__
def __repr__(self):
return str(f'{self._cents/100:,.2f}')
def __str__(self):
return str(f'{self._cents/100:,.2f}')
This is repeated code. You can deduplicate with either
def __repr__(self):
return str(self)
or
__repr__ = __str__
@property
def dollars(self):
return self._cents / 100
@property
def cents(self):
return self._cents
These do not make sense to me. I would expect dollars to return the number of dollars present, and cents to just return the number of cents
c = Currency(7.89)
c.dollars # 7.89, I expected 7
c.cents # 789, I expected 89
You also could implement dollars.setter and cents.setter so you can change just one of them.
Here I will list things I would do differently. That doesn't mean I'm right and you are wrong, it means food for thought, but reject ideas away if they don't suit.
- Use decimal. It was made for arithmetic on numbers where precision matters. It also has the work on dealing with other python numbers pretty much done for you. It also lets you choose how to round which may be important if you need banker's rounding.
- Add a constructor for making an amount from dollars and cents. Something like
Currency(dollars=7, cents=89)
would be nice to use. The implementation is a little trickier (what if cents >= 100, what if dollars is negative).
- Due to the rounding, working with this class may look unfair to whoever owns the money.
Currency(5.05) / 2 * 2 == Currency(5.04) != Currency(5.05)
. By doing what should be nothing, they've lost a cent! To exaggerate the problem, lets say you have Currency(74.49)
to divide amongst 12 people. Currency(74.49) / 12
outputs Currency(6.21)
. You give each person that amount Currency(6.21) * 12 == Currency(74.52)
, and you find you are 3 cent out of pocket. This is when auditors come in and start asking questions.
decimal
\$\endgroup\$/
rather than integer division//
- are you by any chance writing this for Python 2? \$\endgroup\$