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

Make oldFields mapping hardcoded to speed up system #1

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

NikoGrano
Copy link

The mapping came with Magneto 1.6 to provide a backward compatibility
for old (pre 1.6) db field names.
Now the mapping is hardcoded in the class instead of saved
in the configuration.
This improves performance as Magento will not create tons of objects for
every new product/order/... entities.

Related: OpenMage#920

The mapping came with Magneto 1.6 to provide a backward compatibility
for old (pre 1.6) db field names.
Now the mapping is hardcoded in the class instead of saved
in the configuration.
This improves performance as Magento will not create tons of objects for
every new product/order/... entities.

Related: OpenMage#920
@NikoGrano NikoGrano added the enhancement New feature or request label Apr 17, 2020
@NikoGrano NikoGrano self-assigned this Apr 17, 2020
@gocom
Copy link
Member

gocom commented Apr 17, 2020

Only issue could be if some module uses this to alias fields, in which case those modules break.

Maybe this could use a statistically bound proxy class instead? Would still allow configuring aliases, but doesn't init config on each entity instance.

Oh boy, the places where modern DI would be very helpful, because now we would need to store and hard-code it into the entities or the proxy pool that stores the mapping.

@NikoGrano
Copy link
Author

OpenMage#921 (comment)

@NikoGrano NikoGrano merged commit 92d82ae into Lamia:master Apr 22, 2020
NikoGrano pushed a commit that referenced this pull request Dec 16, 2021
…nMage#1403)

* TypeError: round(): Argument #1 ($num) must be of type int|float
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants