Sylius: [RFC] Introduce money value object

Created on 10 May 2016  ·  6Comments  ·  Source: Sylius/Sylius

As discussed in #4087, @michalmarcinkowski mentioned that we might get a proper money value object when working with prices. I think the obvious choice for this is to use: https://github.com/moneyphp/money.

When going down this path, I think in my opinion it should be done before Sylius hits stable.

Some questions:

  1. Should we store the value object as a Doctrine Embeddable (http://doctrine-orm.readthedocs.io/projects/doctrine-orm/en/latest/tutorials/embeddables.html) or simply use a custom mapping type?
  2. When storing the value object, do we only keep track of the base amount, or also the currency?

I think the simplest approach is using a custom mapping type and only storing the base amount as an integer in the database.

It could be as simple as:

class MoneyType extends Type
{
    const MONEY = 'money';

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        return new Money($value, new Currency('EUR');
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        return (int) $value->getAmount();
    }

    public function getName()
    {
        return self::MONEY;
    }
}
RFC Stale

Most helpful comment

@sagikazarmark I removed matthiasverraes/money from dependencies a while ago, it was not used anywhere by then.

Personally I'm 100% for using MoneyPHP in Sylius, but I don't know how it goes along with the roadmap to stable release. Formatters should be no big deal, converters too (using Swap?), but storing the money in database might be. Let's hear from @pjedrzejewski first before starting working on it.

All 6 comments

This is something we are seriously considering, although we have not done any experiments yet. It could totally be embeddable or we could even return new instance in the getter. (you need to know the currency, can be from order, or from current user selection or whatever) If anyone is willing to experiment with such implementation (on some part of Sylius, not entire platform) then I'd be happy to help.

I was in touch with moneyphp guys and I wanted to move our MoneyBundle stuff to this organization, but I miss the time to do this. I'd greatly appreciate any help here. 👍

@pjedrzejewski Any idea if this will be in 1.0? It would be so much better if it would, could you tell me if this is going to be a real thing?

Joining the discussion from MoneyPHP side: as far as I know Sylius already uses MoneyPHP 1.0 (although probably under it's old name mathiasverraes/money).

Regarding the doctrine topic: we are also discussing it in moneyphp/money#328. There are a few options we can choose from, but no decisions yet. If someone could do some actual experiments that would help a lot.

Since 3.0 contains some BC breaks it should be updated in Sylius before it gets the first RC or in 2.0. As the new version contains quite a few improvements and new features (like formatters which could be removed from Sylius) I would recommend taking this step.

I don't know how much time I will have in January, but I will try to spend some efforts on our MoneyBundle.

@sagikazarmark I removed matthiasverraes/money from dependencies a while ago, it was not used anywhere by then.

Personally I'm 100% for using MoneyPHP in Sylius, but I don't know how it goes along with the roadmap to stable release. Formatters should be no big deal, converters too (using Swap?), but storing the money in database might be. Let's hear from @pjedrzejewski first before starting working on it.

ping @pjedrzejewski ;)

We're starting on migrating to moneyphp now, so would be great to hear about any decision on this topic.

In case Sylius would decide to adopt moneyphp that would mean we will not even start looking into other solutions as well.

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings