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

Unable to extend compose(), unable to extend render() #14

Closed
creocoder opened this issue Jul 29, 2015 · 6 comments
Closed

Unable to extend compose(), unable to extend render() #14

creocoder opened this issue Jul 29, 2015 · 6 comments

Comments

@creocoder
Copy link
Contributor

@klimov-paul Because of $this->_message private variable. Also all this $params['message'] workaround looks like real voodoo magic. Maybe its good idea to replace it with something smart?

@samdark
Copy link
Member

samdark commented Jul 29, 2015

What's your proposal?

@klimov-paul
Copy link
Member

$this->_message private variable

Which class scope are you refering exaclty?

Also all this $params['message'] workaround looks like real voodoo magic

It is much magic as $params['widget'] in ListView::renderItem().

What exactly is the problem?

@creocoder
Copy link
Contributor Author

@klimov-paul

What exactly is the problem?

The problem is class can not be extended due to bad design. While it obvious, ok, example:

class MyMailer extends \yii\mailer\BaseMailer
{
    // ...
    public function compose($view = null, array $params = [])
    {
        // my own logic, but i want use $this->render() in my implementation too
        $this->render($view['html'], $params, $this->htmlLayout);
    }
}

In child classes $message in layouts will always be null and you can't do anything with that. Check your code and you'll understand why. Look at original class compose() method:

    public function compose($view = null, array $params = [])
    {
        if (!array_key_exists('message', $params)) {
            $params['message'] = $message;
        }

        // ...
        $this->_message = $message;
        // render call
        $this->_message = null;
        // ...
    }

    public function render($view, $params = [], $layout = false)
    {
        // $this->_message used there and you can't set its value from child classes
    }

Its what i call voodoo magic which make class complettely unextensible. While i understand that you try to avoid override message key if user put it to $params this can be done with pure OOP way without such hacks.

But in general i think its complettely overcoded + ugly hacked. You can just mark message key as reserved in method documentation.

@creocoder
Copy link
Contributor Author

You can just mark message key as reserved in method documentation.

Or there is 10 other methods how to solve trouble without hacks. For example extend render() signature and make $message argument, etc.

@klimov-paul
Copy link
Member

class MyMailer extends \yii\mailer\BaseMailer

Well, in this case this issue does not belong to the yii2-swifmailer repository and should be reported to https://github.com/yiisoft/yii2

Check your code and you'll understand why

First of all it not my code, see:
yiisoft/yii2@a2a6028
So stop blaming, put aside your emotions and concentrate on the issue, please.

The change, which you complain on, has been introduced at yiisoft/yii2#4147
If you have a nice solution for this issue - feel free to sumit a PR.

You can just mark message key as reserved in method documentation.

It was so at the beginning.

@creocoder
Copy link
Contributor Author

@klimov-paul Ok, thanks. Created issue in corresponding repo: yiisoft/yii2#9260

So stop blaming, put aside your emotions and concentrate on the issue, please.

Sometimes its hard to put aside emotions, especially when you see hacks like:

$this->_message = $message;
// call method where $this->_message will be used
$this->_message = null;

Sorry :)

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

3 participants