Why is amount being stored as an integer? This isn't documented, and so it poses problems for other modules, such as Views, that are hooking into this one.

See: http://drupal.org/node/869142.

Any chance we could reroll this as such?

CommentFileSizeAuthor
#2 simple_payments-int_to_decimal-960616.patch4.03 KBseanberto

Comments

seanberto’s picture

seanberto commented
Title: Store amount as a float » Store amount as a decimal
seanberto’s picture

seanberto commented
StatusFileSize
new simple_payments-int_to_decimal-960616.patch4.03 KB

Alrighty, here's a patch for this. It's a bit funky b/c it's applied against the patches in: #839952: Add payer_reference and payee_reference to simple_payment table and #869142: Add views integration. Gosh a lot of this work needs to get into the stable release of this module....Love to help however I can.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Subscribing.

jbrown’s picture

jbrown commented
Status: Active » Fixed

http://drupal.org/cvs?commit=454924

jbrown’s picture

jbrown commented

I think this might have been a mistake.

The advantage of storing the amount as an integer is that you can use normal PHP maths operators. PHP doesn't have a native decimal variable type, so you have to use something like http://php.net/manual/en/book.bc.php to access / manipulate your data.

seanberto’s picture

seanberto commented

I guess that I don't understand the issue. You can present numbers as decimals (or as currency) with standard PHP functions. Can you help me better understand by providing an example scenario where this would cause problems? With the patch I'm using simple payments with Views Calc to provide aggregate payment reports and everything's working okay.

I think that the most obvious problem with storing this data as an integer multiplied by 100 is that it's unintuitive and requires that other modules that hook into yours consistently manipulate the data to make it behave as expected. For example, the fact that you were storing integers makes the view handlers all more difficult.

jbrown’s picture

jbrown commented

The problem is that PHP will treat the amount as a float, but floats can't store base 10 decimals - rounding errors will creep in unless you use bcmath extension.

Putting in a decimal point can be done in the theme layer.

seanberto’s picture

seanberto commented

Huh, Ubercart and the Payments modules both store dollar amounts as floats or decimals in the DB. I don't see the logic just yet re: when they swap back and forth - Ubercart seems to use both in different places. Of the two, it seems to me that a decimal is preferable over a float for the reasons that you've brought up.

Regardless of how you store the value in the DB, it seems like you're always going to have to convert amounts to integers for PHP to handle the math precisely. And even then, you are going to have to be careful that you don't lose precision when PHP tries to convert integers to floats when you do division (see: http://www.php.net/manual/en/language.operators.arithmetic.php). With this in mind, it doesn't seem like you're getting around any of this extra work by saving integers in the DB.

If anything, the DB is the only place where currency can be stored/manipulated sanely as a decimal. Why give that up?

jbrown’s picture

jbrown commented

Okay - I'll leave it as is, but make sure you use bcmath for all mathematical operations on amount.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.