Offer a "Teller" object

[pmjones]

Hi -- in my legacy refactoring work, I have found it useful to introduce a Teller (as in "bank teller") object to ease the transition from float math to Money objects. I wanted to give this project the chance to review, then accept or reject it, before I consider releasing it as a library of its own. The Teller class is pasted below; if you find it suitable for inclusion, let me know and I'll send a PR proper with docs and tests for it. Thanks, hope you are all doing well!

<?php
/**
 * Legacy codebases often use float math for monetary calculations, which leads
 * to problems with fractions-of-pennies in monetary values. The proper
 * solution is to introduce a Money object, and use Money objects in place of
 * float math. However, doing so can be quite an onerous task, especially when
 * the float values need to be moved to and from database storage; intercepting
 * and coercing the float values (often represented by strings) can be very
 * difficult and time-consuming.
 *
 * To help ease the transition from float math to Money objects, use the Teller
 * class to replace float math for monetary calculations in place:
 *
 * ```php
 * // before
 * $price = 234.56;
 * $discount = 0.05;
 * $discountAmount = $price * $discount; // 11.728
 *
 * // after
 * $teller = Teller::new('USD');
 * $discountAmount = $teller->multiply($amount, $discount); // '11.73'
 * ```
 *
 * The main drawback is that you cannot use two currencies for calculations;
 * you can use only one.
 */

namespace Pmjones;

use InvalidArgumentException;
use Money\Currencies\ISOCurrencies;
use Money\Currency;
use Money\Formatter\DecimalMoneyFormatter;
use Money\Money;
use Money\MoneyFormatter;
use Money\MoneyParser;
use Money\Parser\DecimalMoneyParser;

class Teller
{
    /**
     * Static convenience factory.
     */
    public static function new(
        string $currency,
        int $roundingMode = Money::ROUND_HALF_UP
    ) {
        $currency = new Currency($currency);
        $currencies = new ISOCurrencies();
        $parser = new DecimalMoneyParser($currencies);
        $formatter = new DecimalMoneyFormatter($currencies);

        return new static($currency, $parser, $formatter, $roundingMode);
    }

    /** @var ISOCurrencies */
    protected $currencies;

    /** @var Currency */
    protected $currency;

    /** @var MoneyFormatter */
    protected $formatter;

    /** @var MoneyParser */
    protected $parser;

    /** @var int Rounding mode for multiply/divide */
    protected $roundingMode;

    /**
     * Constructor.
     */
    public function __construct(
        Currency $currency,
        MoneyParser $parser,
        MoneyFormatter $formatter,
        int $roundingMode = Money::ROUND_HALF_UP
    ) {
        $this->currency = $currency;
        $this->parser = $parser;
        $this->formatter = $formatter;
        $this->roundingMode = $roundingMode;
    }

    /**
     * Returns a "zero" decimal monetary value.
     */
    public function zero() : string
    {
        return '0.00';
    }

    /**
     * Returns an integer less than, equal to, or greater than zero if a decimal
     * monetary value is respectively less than, equal to, or greater than
     * another.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function compare($value, $other) : int
    {
        $value = $this->convertToMoney($value);
        $other = $this->convertToMoney($other);

        return $value->compare($other);
    }

    /**
     * Are two decimal monetary values equal to each other?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function equals($value, $other) : bool
    {
        return $this->compare($value, $other) === 0;
    }

    /**
     * Are two decimal monetary values not equal to each other?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function notEquals($value, $other) : bool
    {
        return $this->compare($value, $other) !== 0;
    }

    /**
     * Is one decimal monetary value less than another?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function lessThan($value, $other) : bool
    {
        return $this->compare($value, $other) < 0;
    }

    /**
     * Is one decimal monetary value less than or equal to another?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function lessThanOrEquals($value, $other) : bool
    {
        $compare = $this->compare($value, $other);

        return $compare <= 0;
    }

    /**
     * Is one decimal monetary value greater than another?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function greaterThan($value, $other) : bool
    {
        return $this->compare($value, $other) > 0;
    }

    /**
     * Is one decimal monetary value greater than or equal to another?
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value.
     */
    public function greaterThanOrEquals($value, $other) : bool
    {
        $compare = $this->compare($value, $other);

        return $compare >= 0;
    }

    /**
     * Is the decimal monetary value equal to zero?
     *
     * @param mixed $value The decimal monetary value.
     */
    public function isZero($value) : bool
    {
        return $this->equals($value, $this->zero());
    }

    /**
     * Is the decimal monetary value not equal to zero?
     *
     * @param mixed $value The decimal monetary value.
     */
    public function isNotZero($value) : bool
    {
        return ! $this->isZero($value);
    }

    /**
     * Is the decimal monetary value greater than zero?
     *
     * @param mixed $value The decimal monetary value.
     */
    public function isGreaterThanZero($value) : bool
    {
        return $this->greaterThan($value, $this->zero());
    }

    /**
     * Is the decimal monetary value less than zero?
     *
     * @param mixed $value The decimal monetary value.
     */
    public function isLessThanZero($value) : bool
    {
        return $this->lessThan($value, $this->zero());
    }

    /**
     * Adds a series of decimal monetary values to each other in sequence.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value; this param exists
     * to make sure at least one other value is being added.
     *
     * @param mixed[] $others Subsequent other decimal monetary values.
     *
     * @return string The calculated decimal monetary value.
     */
    public function add($value, $other, ...$others) : string
    {
        $value = $this->convertToMoney($value);
        $value = $value->add($this->convertToMoney($other));

        foreach ($others as $other) {
            $value = $value->add($this->convertToMoney($other));
        }

        return $this->convertToString($value);
    }

    /**
     * Subtracts a series of decimal monetary values from each other
     * in sequence.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param mixed $other Another decimal monetary value; this param exists to
     * make sure at least one value *is* being subtracted.
     *
     * @param mixed[] $others Subsequent decimal monetary values.
     *
     * @return string The calculated decimal monetary value.
     */
    public function subtract($value, $other, ...$others) : string
    {
        $value = $this->convertToMoney($value);
        $value = $value->subtract($this->convertToMoney($other));

        foreach ($others as $other) {
            $value = $value->subtract($this->convertToMoney($other));
        }

        return $this->convertToString($value);
    }

    /**
     * Multiplies the decimal monetary value by -1. Note that this will convert
     * negative values to positive ones.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @return string The calculated decimal monetary value.
     */
    public function negative($value) : string
    {
        return $this->multiply($value, -1);
    }

    /**
     * Multiplies a decimal monetary value by a factor.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param float|int|string $multiplier The multiplier.
     *
     * @return string The calculated decimal monetary value.
     */
    public function multiply($value, $multiplier) : string
    {
        $money = $this->convertToMoney($value)->multiply($multiplier, $this->roundingMode);

        return $this->convertToString($money);
    }

    /**
     * Divides a decimal monetary value by a factor.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @param float|int|string $divisor The divisor.
     *
     * @return string The calculated decimal monetary value.
     */
    public function divide($value, $divisor ) : string
    {
        $money = $this->convertToMoney($value)->divide($divisor, $this->roundingMode);

        return $this->convertToString($money);
    }

    /**
     * Returns an absolute decimal monetary value.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @return string The calculated decimal monetary value.
     */
    public function abs($value) : string
    {
        $abs = abs($this->convertToString($value));

        return $this->convertToString($abs);
    }

    /**
     * Converts a decimal monetary value to a Money object.
     *
     * @param mixed $value A decimal monetary value.
     *
     * @return Money
     */
    public function convertToMoney($value) : Money
    {
        return $this->parser->parse((string) $value, $this->currency);
    }

    /**
     * Converts a value into a Money object, then into a decimal monetary
     * string.
     *
     * @param mixed $value Typically a Money object, int, float, or string
     * representing a decimal monetary value.
     *
     * @return string
     */
    public function convertToString($value) : string
    {
        if (! $value instanceof Money) {
            $value = $this->convertToMoney($value);
        }

        return $this->formatter->format($value);
    }
}

[UlrichEckhardt]

I would suggest codereview.stackexchange.com as an alternative place to post the code.

Just formally, i.e. without addressing whether it should be part of this library or not, here are a few notes:

• This seems to be an application of the Facade Pattern, which is worth mentioning in the class docblock and maybe even the class name.
• No space before colon function fname(): returntype (https://www.php-fig.org/psr/psr-12/).
• protected has a similar code smell as public. Make up your mind how you expect derived classes to use protected members and document those. If you don't have an actual plan, make them private.
• The comparison functions are inconsistent in whether they use a temporary $compare variable.
• I would only use a variable args list for add(), not just for the args past the second one. Consider the case of using it to sum up a bill. As it is now, you can't do it if you have less than two items on the bill. Also, $teller->add(..$items) only works in some cases.
• negative() -> negate().
• I have a gut feeling that this would benefit from stronger typing or typechecks. Make sure you don't test only the happy path but actively try to break it with bogus data (it's easy to get null or false from a DB!).
• Run a static code analyzer (e.g. PHPStan) on the code. It will tell you one thing that will make you slap your forehead! ;)

[pmjones]

Thanks @UlrichEckhardt, though I should have specified I was rather looking for feedback from @sagikazarmark, @frederikbosch, or one of the other more senior folks on this project, as to whether this kind of offering was appropriate for the purposes of MoneyPHP in the first place. (If it is, great; if not, that's OK too.)

[sagikazarmark]

@pmjones thanks for reaching out.

I'm definitely not against hosting code that helps the community and does not massively increase the size of this library. I think the above code fits both of those criteria.

@frederikbosch any objections?

[frederikbosch]

No objections. Looks good! It should be tested though. I am planning some major work for this library in the near future. I will include Teller then. If someone wants to provide a PR including tests now, I am not against that!

[pmjones]

@sagikazarmark @frederikbosch You can see a PR with docs and tests at #630 -- let me know if you need anything, and thanks for considering it.

[lukeholder]

This would definitely help us migrate and refactor our product over to use moneyphp/money

Thanks!