shopsys icon indicating copy to clipboard operation
shopsys copied to clipboard

Better extensibility and work with ProductDetailView object

Open pesektomas opened this issue 4 years ago • 2 comments

I think the properties in ProductDetailView should be set using setters and not just by a constructor.

Why do I think that?

Ad 1. ProductDetailView object will have many and many properties during implementation and if we set properties by setters, we would avoid errors of type Argument 37 passed to App \ Model \ Product \ Detail \ ProductDetailView :: __ construct () must be of the type int, string given

Ad 2. I think it could improve the extensibility of the object. The object factory could be have the createInstance andfillFrom *methods and I think if the data was set in thefillFrom *method and you added an attribute there, I wouldn't have to worry about it at all during promotion, because this method would solve it for me. Currently I would have to extend the constructor and add a new attribute to the correct place in the parent :: __ construct call.

Please consider it. I really like reading objects and they make sense to me. With the described modification, it would be possible to work with them even better on the implementation. Or so I think. If you perceive it differently, we can talk about it.

pesektomas avatar Dec 03 '20 21:12 pesektomas

Thank you @pesektomas, currently we are undecided, whether to use this approach. We will contact you to further discuss and argue the "correct way".

sslt avatar Dec 11 '20 08:12 sslt

Thank you, I understand. I see the help wanted label, how could I further help than show an example from a specific implementation? Will we wait for further implementation on the project?

pesektomas avatar Dec 12 '20 10:12 pesektomas

Read-model will not be maintained after version 13.0.0 (FE API replaces it).

Version 12 will be LTS; if it's still relevant for some projects to this version, feel free to create PR.

pk16011990 avatar Jul 28 '23 07:07 pk16011990