-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MoneyHydrator fixed #22
Conversation
@@ -20,7 +20,7 @@ class MoneyHydrator implements HydratorInterface | |||
public function extract($object) | |||
{ | |||
return [ | |||
'amount' => $object->getAmount(), | |||
'amount' => $object->getAmount() / 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is reponsability of hydrator. What do you think about having a custom form element for amount?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only for diving per 100 an extra custom form element is way to much, imho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some currencies that have more than 100 units like INR, so dividing by 100 would result in incorrect values.
Converting to the desired unit is a "view layer" issue. If you're using Zend\Form, that would be your view layer. If you're using API's or SPA, its recommended to return the units, so you convert it in javascript or in the client.
If you get the extracted array and place it into hydrate method, it should return the exactly the same object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First:
I don't know about all currencies, but INR (indian rupee) is definitly not more than 100 units.
The modern rupee is subdivided into 100 paise (singular paisa), see http://en.wikipedia.org/wiki/Indian_rupee.
Second:
Considering dividing 100 or not (with the current module, as it is):
a) You render a form and enter 100 as value and you select USD as currency.
b) money get hydrated and a value of 100 is stored into its money amount property.
c) again 100 is saved to database.
d) last: let's print the value using the given moneyFormat view helper: 1,00 USD ! WTF? I entered 100 USD in a form and get only cents rendered.
Last point: If there is really some currencies out there with more that 100 fractions, it's better to add a getPrecision() method to the currency object itself instead of juggling with divining of 100 in various places.
Best would be, if the module takes care of everything and I don't need to deal with such silly details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want 100 USD, that's 10000 units. You should never persist 100. You should set 10000 in the form. It wil save 10000 in the database and when you return it, you'll have 10000 in money object.
When you return the money object. The MoneyFormat helper already knows about that and does exactly what you suggested, which is dividing it by 100. But that is a "temporary" solution, and its expected from the helper to work with non decimal currencies, since it's in the view layer and this is view layer responsability. http://en.wikipedia.org/wiki/Non-decimal_currency
No description provided.