Allow null amount

[garak]

A null amount is already allowed by lack of type enforcing, but phpdoc is suggesting that is not. I think it should be allowed, so when types will be enforced this can be taken into account.

[frederikbosch]

We will not allow a null amount.

[garak]

We will not allow a null amount.

Well, you're already allowing it. There's no check to avoid a null amount. Anyway, how do you suggest to deal with a Money that is nullable?

[frederikbosch]

Well, you're already allowing it. There's no check to avoid a null amount.

Sorry, I closed your issue too early. You are right. I added a simple test in #618 that proves your point.

Anyway, how do you suggest to deal with a Money that is nullable?

Outside Money, e.g. make the variable that can hold a Money object nullable.

[UlrichEckhardt]

I'd second this. PHP null is not a meaningful value when combined with a unit (the currency), it is in particular not the number zero. Think about how a money object would be used when its amount could be any number plus null. In all code handling this, you'd have to create an extra case for that, which weakens the guarantees given by the money class significantly.

If you wanted to argue your point, please consider creating example code that would benefit in any way from having a null money value.

[garak]

Here is my use case: I map a Money to a Doctrine value object. In my domain, the amount of money can be nullable (I guess it's a pretty common scenario). Is there a way to apply such mapping with a null Money? The only way I found so far is to map to a Money object with a null value.

[Padam87]

@garak Did you find a way to do this with embeddables? Right now settling for a custom type instead seems to be the best way around it.

[garak]

@Padam87 the way I found is the one described above: a Money object with a nullable value

[garak]

@Ocramius since this has been solved not allowing null anymore, do you have a suggestion on how to properly map an embeddable in Doctrine?

[Ocramius]

There are no nullable embeddable values in doctrine

On Sun, May 2, 2021, 16:20 Massimiliano Arione @.***> wrote:

@Ocramius https://github.com/Ocramius since this has been solved not allowing null anymore, do you have a suggestion on how to properly map an embeddable in Doctrine?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moneyphp/money/issues/615#issuecomment-830816944, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEAAQB3MD57K3OTVOHLTLVNS3ANCNFSM4R6Z3PEA .

[garak]

There are no nullable embeddable values in doctrine

I see. So, how do you use this lib in Doctrine?

[Ocramius]

I map the embeddable directly: if null is needed, then I build my own Money instances at runtime.

On Sun, May 2, 2021, 16:26 Massimiliano Arione @.***> wrote:

There are no nullable embeddable values in doctrine

I see. So, how do you use this lib in Doctrine?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moneyphp/money/issues/615#issuecomment-830817833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEBPFUQJP5AEFD266C3TLVOK3ANCNFSM4R6Z3PEA .

[garak]

I map the embeddable directly: if null is needed, then I build my own Money instances at runtime.

OK, but how do you map it? How can you avoid to map the amount property without nullable attribute? Just to be clear, this how I map it currently (and it looks like it won't be able to do it anymore):

<embeddable name="Money\Money">
    <field name="amount" type="integer" nullable="true"/>
</embeddable>

[Ocramius]

I map two separate nullable fields, then construct the Money instance myself

On Sun, May 2, 2021, 16:53 Massimiliano Arione @.***> wrote:

I map the embeddable directly: if null is needed, then I build my own Money instances at runtime.

OK, but how do you map it? How can you avoid to map the amount property without nullable attribute? Just to be clear, this how I map it currently (and it looks like it won't be able to do it anymore):

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moneyphp/money/issues/615#issuecomment-830821537, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEA3HO2QVT247L4MOSTTLVRM7ANCNFSM4R6Z3PEA .

[garak]

I map two separate nullable fields, then construct the Money instance myself

So, are you keeping two additional properties on your entity, only meant to be mapped (and to be used to build Money)?

[Ocramius]

Yes

On Sun, May 2, 2021, 17:56 Massimiliano Arione @.***> wrote:

I map two separate nullable fields, then construct the Money instance myself

So, are you keeping two additional properties on your entity, only meant to be mapped (and to be used to build Money)?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moneyphp/money/issues/615#issuecomment-830830782, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEGNDARY5B2RGOUSEYTTLVY2LANCNFSM4R6Z3PEA .

[Padam87]

This uses brick/money, but the idea is the same:

class NullableMoney
{
    /**
     * @ORM\Column(type="decimal_object", precision=28, scale=4, nullable=true)
     */
    private ?BigDecimal $amount = null;

    /**
     * @ORM\Column(type="currency", nullable=true)
     */
    private ?Currency $currency = null;

    public function __construct(?Money $money = null)
    {
        $this->amount = $money ? $money->getAmount() : null;
        $this->currency = $money ? $money->getCurrency() : null;
    }

    public function __invoke(): ?Money
    {
        if ($this->amount === null || $this->currency === null) {
            return null;
        }

        return Money::of($this->amount, $this->currency);
    }
}

    public function get(): ?Money
    {
        return ($this->money)();
    }

    public function set(?Money $money): self
    {
        $this->money = new NullableMoney($money);

        return $this;
    }