Skip to content
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

Rounding issues #101

Open
zirnipa opened this issue Mar 23, 2021 · 1 comment
Open

Rounding issues #101

zirnipa opened this issue Mar 23, 2021 · 1 comment
Labels

Comments

@zirnipa
Copy link

zirnipa commented Mar 23, 2021

Bug reported by Leonidas D.

mpay24-php / src / Requests / ManualCredit.php

Methode setAmount():

/**
 * @param int $amount
 */
public function setAmount($amount)
{
    $this->amount = (int)$amount;
}

Das TypeCasting das hier via (int) stattfindet kann zu unerwarteten, forcierten Rundungsergebnissen führen, zumindest wo ich es getestet habe.

https://stackoverflow.com/questions/20758881/why-is-php-typecasting-behaving-like-this-int-mis-rounding-numbers

So z.B. ein Fall mit:

$amount=17.90;
// $amount ist hiermit ein float

$mpay24->refund(‘00000000000’, ($amount * 100));
// Übergeben wird nun zwangsläufig ein float, aber halt ohne Nachkommastellen

public function setAmount($amount)
{
$this->amount = (int)$amount;
// $this->amount ist nun 1789
}

Das ist wie es bei den Gutschriften zum Rundungsproblem kam. War halt völlig unerwartet und tauchte beim Test nicht auf, denn interessanterweise gibt es Beträge wo das nicht passiert. 25,90 => kein Problem. 19,20 => kein Problem. 17,90 => Problem!

Natürlich kann man dem entgegenwirken indem man schon vorher einen Integer übergibt. Aber vielleicht wäre in den Funktionen die den Amount annehmen die Erwartung einen Integer zu erhalten besser als es nachher zu typecasten:

/**
 * @param int $amount
 */
public function setAmount(int $amount)
{
    $this->amount = (int)$amount;
}
@zirnipa zirnipa added the bug label Mar 23, 2021
@stefanpolzer
Copy link
Contributor

stefanpolzer commented Mar 25, 2021

@zirnipa As part of the docBlock we define that the integrator has to give the right value as integer.
Because the API request an integer, see: https://github.com/mpay24/mpay24-php/blob/master/src/Mpay24.php#L221

The real question is who is responsible to converting the value into an integer. IMHO this is the integrator.
We only guarantee that the value will be sent to the API as an integer value.

If done right this should work. A quick possibility is to add a fraction

$amount=17.90;
$mpay24->refund(‘00000000000’, (($amount + 0.0001) * 100));

Of course we could apply the same logic within your code as well:

/**
 * @param int $amount
 */
public function setAmount($amount)
{
    $this->amount = (int)($amount + 0.0001);
}

BUT adding a type hint into the method will have no effect if we get a flaot, because PHP is doing here the same as with the cast:

<?php

class Foo
{
    public function set(int $int){
        echo $int;
    }
}

$foo = new Foo();
$foo->set(17.90 * 100);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants