-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Field mapping DTO #10607
Field mapping DTO #10607
Conversation
We are going to have many DTOs, all of which will need an array access implementation as a backward compatibility layer.
fcda732
to
c0fe182
Compare
In the past, it has been decided to use arrays for this out of legitimate performance concerns. But PHP has evolved, and now, it is more performant and memory efficient to use objects.
4f9ba82
to
175cdef
Compare
@@ -257,7 +257,7 @@ private function formatMappings(array $propertyMappings): array | |||
foreach ($propertyMappings as $propertyName => $mapping) { | |||
$output[] = $this->formatField(sprintf(' %s', $propertyName), ''); | |||
|
|||
foreach ($mapping as $field => $value) { | |||
foreach ((array) $mapping as $field => $value) { |
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.
I assume having an IteratorAggregate would be too much and is supposed to be kept simple.
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.
Yes, and it's not like this is a collection or something.
Thank you for the effort of splitting up the former PR. ❤️ |
The requested changes have been addressed.
In the past, it has been decided to use arrays for this out of
legitimate performance concerns. But PHP has evolved, and now, it is
more performant and memory efficient to use objects.