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

Ability to set default context and rounding mode #59

Open
Padam87 opened this issue Apr 13, 2022 · 3 comments
Open

Ability to set default context and rounding mode #59

Padam87 opened this issue Apr 13, 2022 · 3 comments
Labels

Comments

@Padam87
Copy link

Padam87 commented Apr 13, 2022

I would like to open a discussion about the possibility of implementing a way to override the context, and rounding mode of money methods.

These are hard coded all the way, and I personally find it pretty annoying, and ugly that I have to specify what I would like to do.
99.99% of the time I use my own context, and a RoundingMode::HALF_UP.

This is because in real use case scenarios the money object is always constrained by the database schema.

For example: We use 6 decimals because we work with really small, really preciese values.
Let's say the price of a single sheet of paper is 0.123456 EUR. Rounding that to 0.12 and storing it is a no-go, considering you literally need millions of these printed sometimes, and that could make a huge difference - a loss.

Calculations are always done in Rational, and cast to Money at the very end, to avoid rounding as much as possible of course, but the end is always the database scale and precision.

Long story short: DefaultContext is not viable for everyone.

public static function of($amount, $currency, ?Context $context = null, int $roundingMode = RoundingMode::UNNECESSARY) : Money
    {
        if (! $currency instanceof Currency) {
            $currency = Currency::of($currency);
        }

        if ($context === null) {
            $context = new DefaultContext();
        }

        $amount = BigNumber::of($amount);

        return self::create($amount, $currency, $context, $roundingMode);
    }

Could we make some changes here that would allow global configuration?
(This might be the rare case when I'm not against the use of a singleton for configuration)

Proposal:

public static function of($amount, $currency, ?Context $context = null, int $roundingMode = null) : Money
    {
        if (! $currency instanceof Currency) {
            $currency = Currency::of($currency);
        }

        if ($context === null) {
            $context = Configuration::getInstance()->getContext();
        }

        if ($roundingMode === null) {
            $roundingMode = Configuration::getInstance()->getRoundingMode();
        }

        $amount = BigNumber::of($amount);

        return self::create($amount, $currency, $context, $roundingMode);
    }
@nufue
Copy link

nufue commented Apr 17, 2022

@Padam87 I wonder whether this would not be better solved by having a factory method in your code, which would create Brick\Money instances with desired context and rounding mode, rather than introducing a static configuration to the library.

@Padam87
Copy link
Author

Padam87 commented Apr 17, 2022

@nufue Unfortunately that is not a way to go here.

First off, this is a library problem, I always prefer a built in solution for these.

Second, the of static method is just an example here. Virtually all methods hardcode these, so I wouldn't be able to use any of those.
Casting a rational to decimal money the overhead a factory provides is actually worse than specifying context and rounding mode like I do now.

So no for the factory, mainly because it is not useful for anyone wanting to use the built in methods, eg plus, minus etc

@Padam87
Copy link
Author

Padam87 commented Apr 17, 2022

BTW, there is already an issue here, #38

That would also be a solution for this, as I could add a shorthand for myself.

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

3 participants