Mage Factory Methods: Direct "_registry" Access Without Method Call #2319
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Direct access is possible because null is no possibility due to executed code prior. This avoids additional isset checks being unnecessary.
Sdfendor
commented
Jul 15, 2022
…e::getSingleton().
…Mage::getSingleton(). Added explanation to type hints as well.
github-actions
bot
added
Component: Catalog
github-actions
bot
removed
Component: Adminhtml
fballiano
approved these changes
Jul 20, 2022
elidrissidev
approved these changes
Jul 20, 2022
elidrissidev
pushed a commit
to elidrissidev/magento-lts
that referenced
this pull request
Jul 22, 2022
* refactor: Accessed _registry directly in several Mage factory methods. Direct access is possible because null is no possibility due to executed code prior. This avoids additional isset checks being unnecessary. * refactor: small code formatting adjustments * refactor: Utilized null coalescing operator. * fix: Try so solve PHPStan complaint due to changed return type of Mage::getSingleton(). * fix: Solved rest of PHPStan complaints due to changed return type of Mage::getSingleton(). Added explanation to type hints as well. * fix: Reverted automatically replaced short list syntax due to php 7.0 compatibility. * Revert "fix: Solved rest of PHPStan complaints due to changed return type of Mage::getSingleton()." This reverts commit c9d165c. * Revert "fix: Try so solve PHPStan complaint due to changed return type of Mage::getSingleton()." This reverts commit fc9901c. * fix: Reverted return type of getSingleton to original state.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description (*)
Four factory methods in the main Mage class use the
registry()method to retrieve object instances they created prior. To use this method in those cases is unnecessary and counterproductive. This PR adjusts those methods to useself::$_registrydirectly.In addition small formatting fixes are applied and one null coalescing replaces a "if/isset" construct. Due to a changed return type of
getSingleton()a few type hints (with explanation) are added as well to satisfy PHPStan static analysis.Questions or comments
registry()calls be replaced?The method accesses
self::$_registrywhile checking if givenkeyexists and is not null. In each adjusted case however it is guaranteed by the code executed prior that the used$registryKeyis not null. Takehelper()for example:The value of
self::$_registry['registryKey']is either an object instance (the helper) or a fatal error is emitted because class name in$helperClassis not found. Null isn't possible and the registry value is written.Contribution checklist (*)