-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Ease immutable/value objects mapping #19367
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
Conversation
While I do not disagree (nor agree) with the PR, I would advise against Value Objects in forms, as much as Entities. A simple DTO which is allowed to be in an invalid state, is a lot easier and you can simply populate whatever data you need with the DTO. http://stovepipe.systems/post/avoiding-entities-in-forms This requires no weird data object filling callbacks or what so ever to be achieved. I suggest people to keep it simple and keep it decoupled. |
At first, we used simple DTO objects (with getter/setter or public properties), more by being constrained by Symfony forms & other Infrastructure stuff than by conviction. In our projets, we're using a lot the command bus, and we often delegate the creation of the Command/Query instance to form. Thus, forms are in charge to extract data from the request, create the command/query instance, and finally, validate it before passing it to the bus. The latter being in charge to handle the command and reflect the changes in our entities. Value objects, on another hand, brings much more flexibility and reusability for simple objects across the application, and does not require any intermediate DTO, but can allow to directly map our model to the forms (encapsulated in a command or not). There is no such things as risky as with entities, because they are value objects, are not directly mapped to our entities, and are a strong echo to our model in each layer of the application. Now, I agree with your comment, and I actually think both approaches are fully valid, but can be preferred according to different use cases, or simply by taste/conviction. I hope this make sense to you and other folks and would help considering new features in this direction. 😃 Finally, I don't understand why you'd advise against Value Objects in forms as much as Entities ? (BTW, I know about your blog post, and I'm 👍. Great post.) |
For the exact reason you've just mentioned:
Take the following example of a Value Object: If you allow this, your |
Transformers and specialized form types would not allow such a thing, and even if it was the case, you're able to throw a |
Which would not be a graceful error handling. If you truly want your
|
A
I don't get your point. Mapping an object that won't need to be validated itself by an external component (it may be validated according to the context, though) should not prevent me to use Symfony Forms. It's not the only data submitted. But anyway, mine is not only about value objects, but about the fact I don't want either to design my commands and Application stuff according to Infrastructure limitations. ...But of course, only if it makes sense to some majority 😄 |
@iltar |
@unkind : Just to be clear about your pretty good observation: a command should be an immutable object. Not particularly a value object (which conveys an even stronger signification). |
I consider command message as a particular case of VO. We don't allow to create invalid VOs, because they don't make sense. |
Interesting. That's exactly where I put the boundary (at least one of the hints) between a simple immutable object and a value object. 😅 To me, a value object is validated by itself, as a self-sufficient object and has a strong meaning about the value it encapsulates. So I guess we actually substantially agree, but do not share the exact same definition. |
By analogy, you can compare command message with HTTP-request message. |
I do have to note that in the explanation I gave, you can compare the DTO to a Command; E.g. However, with (Symfony) forms, you want to add violations and not throw errors/exceptions. For example: __construct($amount, $currency)
if !is_number $amount: throw new InvalidArgumentException; This would prevent the object from ever be in an invalid state (which is what you've indicated). If you were to simply accept every value in the constructor, that means your object will not be able to be "valid" in a way you can ensure it upon creation. This is what I mean with:
If you throw an exception, you can't add it as error message for your form. If you don't throw the exception, it can be created in an invalid state. As @unkind mentioned immutable objects, then it does make sense to solve it the way you propose. There's a big difference between an immutable object and an object that always has to be valid. For me this method would be extra overhead. If I have a I would also like to mention that as a DTO, I fill data in 2 places. As this object contextually transfers data, I also set data from the controller, for example the subject in question. This comes from the Action arguments for example, which went through authorization. If I have to add this contextually as subject, I have to call a setter, making it impossible to make this object immutable or always in a valid state. Or the other way around, my DTO cannot exist without a subject, thus I have to create the object with the subject as constructor argument. My Form will make sure the DTO is validated and once done, my So, to make this story short, I think it's a lot of extra effort to create your objects this way. However, I do think there are use-cases if immutability works for both your forms and your default data. For objects that have to always be in a valid state, this won't work due to the DTO being in an invalid state from raw data. |
I use command bus too. But my commands are actually just simple DTOs validated by validator component and mostly constructed and "filled in" by form component. They are not immutable. "A Command is a message that represents the intention of the user. Command validation happens after the form is submitted and before the Command is passed to the model." http://verraes.net/2015/02/form-command-model-validation/ OFC the commands could be immutable but then I couldn't easily use them as DTOs and would need to create an additional ones. In my controller I don't need to do any more logic than passing the command to command bus. And the commands have their intention mor clear in their name. For example And it's a little bit OT but in my value objects i throw validation exeptions (not TransformationFailedException , I've prepared a special interface for them - only for those in my domain) and I then catch it using a special form extension which translate the exception messages and fills the form errors based on those exceptions (you want to propagate only some of them) to the form. I am +1 (but ok i am just an ordinary person passing by :)) for somehow ease immutable/value objects mapping, not sure if particulary for this PR, not against :). The current way of passing the data array by reference is a little bit weird. But at least it works. The callback option could be a way. |
@iltar I agree with you on value objects like Moreover, if my API is command-oriented, I probably don't need the Form component at all. My message was about terminology. It looks like we have a little bit different opinions on whether command is a value object, but it's not that important. |
Here is a test case showing how behaves value objects in forms if you're using proper form types for building the value object, or if you throw yourself a custom Now I'm not advocating about binding value object to your forms or not. I just say it is possible and can be gracefully handled. |
@ogizanagi That's a nice feature! Thanks for proposing it in the core. I've already thought about using a DataMapper as a simple callable. Especially after I've read a long time ago what makes me saying now that this option looks very close to the Shouldn't we use the same logic in both cases, whether submitted data is Actually the |
@HeahDude : Thanks for sharing your opinion on this. 😃
I'm not sure about what you meant ? If you actually meant the Your reference makes me think I'd like the idea of having the |
Not talking about the logic of performing validation on VOs via Forms but, the current way being quite annoying, the feature sounds really nice to me. Edit: If throwing a |
Hi @webmozart :) May I ask you your opinion on this feature ? |
by adding a new "simple_object_mapper" FormType option.
@HeahDude: Do you think this feature has any chance to have a future in the Symfony core or is it too marginal? Does it need more flexibility or anything to be enhanced? |
Hey @ogizanagi, I understand the motivation behind this PR, and I'm not fundamentally against it (as I said before), but I think the current implementation could be simplified a lot; I don't think we really need new interface(s) to handle it for example. Anyway before you keep working on this one, I'd like to sanitize the current implementation of the component first by refactoring some parts, and maybe solve this use case in the process. Have you already read #8834? So I propose to come back later on this, besides you made a bundle for it ;). So if anyone wants to use/test it, I guess he can :D. |
Great. I'm looking forward to seeing what you're going to produce :) |
interface FormDataToObjectConverterInterface | ||
{ | ||
/** | ||
* Convert the form data into an object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{@inheritdoc}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface does not extend any other interface.
interface ObjectToFormDataConverterInterface | ||
{ | ||
/** | ||
* Convert given object to form data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converts.
@HeahDude @ogizanagi What's the status here? Looks like @HeahDude was going to propose something, but... |
I think we can close as it does not seem to get much attraction. If anyone is interested again, they can give a try to this package first. We can still consider reopening it later. |
@ogizanagi sounds like a good plan. Let's close then. Thanks! |
Delegating this to a package which is dead on arrival is odd though. But yeah, at least you can take a peek and copy things you need out of it. |
Or provide a PR to update the Sf compatibility ;) |
by adding a new
SimpleObjectMapper
data mapper andsimple_object_mapper
FormType option.This new feature is based on the great @webmozart 's blog post about "Value Objects in Symfony Forms", but tries to expose a simpler API to ease value objects & immutable objects manipulations with Symfony forms.
Show code
Considering the following value object:
Before
After
After (with shortcut)
with
simple_object_mapper
option as a callableShow basic API (without using the form option)
Setting the data mapper yourself:
Using the
CallbackFormDataToObjectConverter
:I make this suggestion in order to ease value objects & immutable objects manipulations in forms, considering the fact most of the time, we don't need to bother about implementing the
DataMapperInterface::mapDataToForms()
method. We can simply reuse thePropertyPathMapper
implementation.Then, only the
DataMapperInterface::mapFormsToData()
method is useful. But once again, we don't really need to deal withForm
instances, only with form's data, in order to describe to our form type how to create a new value object instance.For most advanced usages, an
ObjectToFormDataConverterInterface
interface can also be implemented, allowing to skip the original mapper (in most cases thePropertyPathMapper
) implementation, to simply map the data to the form ourself, by converting to value object to an array of form data indexed by field name.Show `ObjectToFormDataConverterInterface` implementation sample
Now, given the following two facts:
I suck at naming things, and I'll gladly consider any better name for the followings:
FormDataToObjectConverterInterface
andObjectToFormDataConverterInterface
interfaces and methods.SimpleObjectMapper
data mapper name andsimple_object_mapper
option.Note: I originally made this available as a
FormType
extension for one of our projects. But considering the DDD and clear DTOs, Model & Entity separation vibes, I think it makes sense to have this new mapper in theSymfony\Form\Extension\Core
namespace.As a solution until a decision is made here, the code is available in a dedicated repository: https://github.com/Elao/FormSimpleObjectMapper