The "ControlSum" and the "InstructedAmount" value miscount about php-sepa-xml HOT 11 OPEN

Yeah you a right, I pushed a commit to my fork with a test examining the failure. While fixing this I stumbled upon another issue.
If you provide 19.999 as amount this will sum up to 20.00 at the moment. I´m not sure about how to handle this. I think it not the responsibility of a xml writer to round your amount. But on the other hand this lib is already calculating the sums of the whole Transfer which can´t be done without bringing the numbers to a common base.

Any Ideas ?

from php-sepa-xml.

Today discussed at worked the case of casting floats. Its terrible;

There is something internally happen when casting from string to double that is not happening when casting from integer to double.

php > $d = '19.99' * 100;
php > $d1 = (double)1999;
php > var_dump($d);
double(1999)
php > var_dump($d1);
double(1999)
php > var_dump((integer)$d);
int(1998)
php > var_dump((integer)$d1);
int(1999)

The following is explaining this a little.
php > printf('%10.20f', $d);
1998.99999999999977262632
php > printf('%10.20f', $d1);
1999.00000000000000000000

I think a possible solution is to only allow cent amounts and only in the xml generation a conversion to a float representation is done.

My experience with calculating prices is to always use cent amounts and only when displaying prices convert them to float representations.

So my idea is to only accept cent amounts as this is done internally already there should be no problem in the internal calculation.

from php-sepa-xml.

I have not yet had a chance to look at the actual code, but just to throw in an idea, another way could be to use the bcmath extension when available. It allows precision math operations on values that are converted from strings:
http://de1.php.net/manual/de/function.bcadd.php

from php-sepa-xml.

Yes, the right way is to use bcmath Methods: http://www.php.net/manual/en/ref.bc.php

from php-sepa-xml.

Isnt it more stable to only use cents then its not the responsibility of this library to calculate correctly as this should be done in the software using this library.

from php-sepa-xml.

For me this litle changes are working:

File: vendor/digitick/sepa-xml/lib/Digitick/Sepa/TransferInformation/BaseTransferInformation.php

Line 83 and up:

/**
 * @param mixed $amount
 * @param string $iban
 * @param string $name
 */
public function __construct($amount, $iban, $name)
{
    bcscale(2);
    $this->transferAmount = bcadd($this->transferAmount, bcmul($amount, '100'));
    $this->iban = $iban;
    $this->name = StringHelper::sanitizeString($name);
}

from php-sepa-xml.

@ianare Hey, I think this can be closed, right ?

from php-sepa-xml.

Not sure if it can be closed - I just encountered the issue again using the latest trunk on PHP 5.3.3/Gentoo, one cent is missing in a transaction of originally 65.59€ (control sum is calculated correctly).

I was unable to reproduce this issue on PHP 5.4/Mac so far, so at the moment I'm guessing it's a bug in PHP 5.3.3, but this could just as well mean that float conversion is simply unreliable depending on either PHP version or platform, and all calculations should always be done in bcmath or using integers.

Regarding the current bcmath handling, currently there's a check if a parameter is passed as float and then bcmath will be used. Technically, this is incorrect, as bcmath accepts strings as input - so a type conversion will take place before the value is passed to bcmath. Worst case the type is cast twice (comes from data store as string, cast to float to activate bcmath processing in php-sepa-xml, cast back to string to pass on to bcmath).

I'll try and pinpoint the problem further.

from php-sepa-xml.

I think there is a coherence probleme with the way you fix the problem.

If i'm not wrong this code means the amount is in cents, if its a int, and if it's float it's not in cents ?
That right ? Correct me if i'm wrong ?

I have to use the script in production and i'm not confident to use it this way, i use int amount in cents (beacause no bcmath on my php conf)

from php-sepa-xml.

Hi @Patafix, as mentioned in my second comment I think the solution is to only allow cents. But other thought it would be better to also allow cent amounts. I understand your concerns about using it in production. If you have a good Idea or a patch don't hesitate to send a pull request or a comment here.

from php-sepa-xml.

I don't hesitate, i just want to check if i have understood correctly how the amount work (int => CENTS, FLOAT => NO CENTS) before use it.
Thank for you work.
I think your solution of allow only cent is the good one, personnaly i send the amount only in cents.

from php-sepa-xml.