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

More access to field definition and data in custom types #7303

Open
Padam87 opened this issue Jul 9, 2018 · 9 comments
Open

More access to field definition and data in custom types #7303

Padam87 opened this issue Jul 9, 2018 · 9 comments

Comments

@Padam87
Copy link
Contributor

Padam87 commented Jul 9, 2018

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

Summary

This is mainly a question, but it may become a feature request.

We use and store a lot of money https://github.com/moneyphp/money objects. These are tied to the same entity, and their currency is always the same.

Right now, there are 2 solutions to do this.

1. Create an embeddable mapping for Money\Money, and store it directly.
This is nice and clean on the PHP side, and if you only have one money field in your entity, it is perfect. When you have 10, you are going to end up with a table with 10 amount, and 10 currency fields.

2. Store the currency, and the 10 amounts; instanciate the Money objects in the getters.
This way, you get a nice database, but a really messy Entity, with a lot of problems:

  • Getters instanciating the Money object.
  • Embeddables don't have access to their owners, so if you have amounts stored there, you have no access to the currency (possible to "fix" with postLoad listener inject).
  • Setters problems: You either only accept Money objects (to keep consistency with the getter), and deal with the conversion, or you can accept bigint / Money (to simpify usage).

I know this is a very specific problem, and the doctrine library is not supposed to solve it.
However, we developers should be able to create an adequate solution.

I wanted to create a custom type to solve this, but unfortunately I couldn't.

  • convertToPHPValue has no access to other values (even raw values would be fine here)
  • Also tried to use convertToPHPValueSQL to concatenate the fields into something like EUR 10000, but this method has no access to the field definition (I wanted to set the name of the currency field in the options)

Then I started thinking about using a postLoad listener to change the bigint values to Money. This would have worked, but I don't really feel good about it, as it borders on black magic territory.

So I went with option 1, mainly because it will be easy to refactor once I have a real solution.

Could it be possible to improve Types in Doctrine3? Give some more context to the methods mentioned above?

@Padam87
Copy link
Contributor Author

Padam87 commented Jul 9, 2018

I would be curious what @sagikazarmark and @frederikbosch think about this.

@frederikbosch
Copy link

@Padam87 I don't use Doctrine, but use a similar solution as number 1. Why is it a problem to have 20 database fields for 10 value objects? I mean the number of columns is hardly limited (4096 in case for mysql).

@Padam87
Copy link
Contributor Author

Padam87 commented Jul 10, 2018

It is more a "dislike" than a problem. We could argue redundancy, but since it is 3 chars long I wont. 😄

Currency is always the same for the 10 amounts. The way to store this normally is 1 currency field, and 10 amounts.

Embeddables should not do that (obviously). Custom types should be able to, but they cannot due to the lack of context in the 2 methods mentioned above.

@frederikbosch
Copy link

@Padam87 If you ask me, you are solving a problem that does not exist. I don't see why you want to spend time in reducing the number of columns in a database.

@Padam87
Copy link
Contributor Author

Padam87 commented Jul 10, 2018

Hm, maybe it's just me, but just because I use an ORM, I don't ignore my database schema.

@frederikbosch
Copy link

@Padam87 That's not the point. I totally agree with you that using an ORM should never mean ignoring the database schema.

However, a table can simply contain some overhead with the same data in different columns. Otherwise you should create a table currencies and then have all currencies in that table and reference it with a currency_id. I think you are trying to abstract something that should not be abstracted.

@Ocramius
Copy link
Member

Mapping the same field to multiple values is out of discussion anyway, but mapping N distinct fields to M distinct value objects is something that we are considering for ORM v3.

@Padam87
Copy link
Contributor Author

Padam87 commented Jul 10, 2018

@Ocramius Would that cover mapping currency + amount1 to money1 and currency + amount2 to money2?

It would be perfect. :)

@Padam87
Copy link
Contributor Author

Padam87 commented Jul 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants