lajax / yii2-translate-manager

Translation Manager
MIT License
227 stars 90 forks source link

Use getModule() instead Module::getInstance() #86

Closed ilgiz-badamshin closed 7 years ago

giannisdag commented 7 years ago

I also agree that this is required, but it is better to use $module = \Yii::$app->controller->module; because the proposed solution getModule assumes that the user uses a specific name for the module which is not always true.

moltam commented 7 years ago

Even better solution: $this->controller->module.

ilgiz-badamshin commented 7 years ago

Fixed it. But Yii::$app->getModule('translatemanager') is still used 8 times.

moltam commented 7 years ago

Is there a better way to access a module when not in a controller context (for example in a widget, or component)? I am not aware of it. I don't think we can remove all of the occurrences.

insane-dev commented 7 years ago

May be use trait for that? e.g.

namespace lajax\translatemanager\traits;

use Yii;
use lajax\translatemanager\Module;

trait ModuleGetterTrait
{
    private $_module;

    public function getModule()
    {
        if ($this->_module === null) {
            $this->_module = Module::getInstance() ?: Yii::$app->getModule('translatemanager');
        }

        return $this->_module;
    }
}

And then when you want to give developers ability to change the name of the module in app config, you need to change only one method in one trait that is used all over the module.

Other ideas:

  1. When user changes module name in app config, then advice him from module docs to set new module name in Application::$params property, and check that property in trait.
  2. You can check context in trait:
if (property_exists($this, 'controller')) {
    $this->_module = $this->controller->module;
}
lajax commented 7 years ago

Thanks!