[TASK] Add trait for getArrayRepresentation to populate class - #1451
Conversation
29ada4a to
8f8f152
Compare
8f8f152 to
5f41933
Compare
5f41933 to
0f02a73
Compare
|
Sorry, I missed that this is the PR to review and that #1452 is a draft PR. I'll copy my comments over. |
| * | ||
| * @internal | ||
| */ | ||
| public function getArrayRepresentation(): array |
There was a problem hiding this comment.
I'd strongly prefer the trait to not have an implementation of getArrayRepresentation as this can lead to classes with an incomplete implementation if they implement the trait. (Also, I'd like to avoid parent:: calls.)
Instead, I suggest having a private method getShortClassName(): non-empty-string in the trait. Then we can also build the array from scratch in getArrayRepresentation in the implementing classes, allowing for more concise code.
There was a problem hiding this comment.
Instead, I suggest having a private method
getShortClassName(): non-empty-stringin the trait.
Changed as suggested. Also renamed the trait accordingly.
(Also, I'd like to avoid
parent::calls.)
Then we can also build the array from scratch ingetArrayRepresentationin the implementing classes, allowing for more concise code.
In cases where a subclass includes additional properties and the parent also has some properties, it would be more concise not to duplicate filling in the parent's properties in the subclass's implementation.
Part of #1440.