mpay24 / mpay24-php

Offical mPAY24 PHP SDK
MIT License
12 stars 15 forks source link

Rounding issues #101

Open zirnipa opened 3 years ago

zirnipa commented 3 years ago

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;
}
stefanpolzer commented 3 years ago

@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);