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

Decrease dependencies from Mage class #15

Closed
stalniy opened this issue Apr 27, 2012 · 3 comments
Closed

Decrease dependencies from Mage class #15

stalniy opened this issue Apr 27, 2012 · 3 comments

Comments

@stalniy
Copy link

stalniy commented Apr 27, 2012

Why do you use Mage::helper instead Mage_Core_Block_Abstract::helper (e.g. the same file at line 191)? Using the last one choice you will get more flexibility.
Also it will be nice if you create a getConfigValue method instead of using Mage::getStoreConfig.

Then, in future, when you want to implement dependency injection pattern it will easier.

@magento-team
Copy link
Contributor

Hi @stalniy,

Mage_Core_Block_Abstract::helper($name) calls $this->getLayout()->helper($name), which refers to the same Mage::helper($name).

Actually Mage::getStoreConfig() returns store config value.

@stalniy
Copy link
Author

stalniy commented May 1, 2012

Hi. Yes i know this. But if you use Mage_Core_Block_Abstract::helper everywhere (where it's possible) instead of Mage::helper then in future it will be easy to improve architecture. Because you will need to change only one method in Mage_Core_Block_Abstract class and only in one file.

@magento-team
Copy link
Contributor

You are right. It's easier if Mage_Core_Block_Abstract::helper() method should do something different from Mage::helper(). Thank you for the advise, we'll consider it, if Mage_Core_Block_Abstract class will be refactored.

magento-team pushed a commit that referenced this issue Jan 16, 2015
…on-save-path

[Extensibility] Magetwo 31851 session save path
@chrom chrom mentioned this issue Mar 4, 2015
@FabXav FabXav mentioned this issue Oct 11, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants