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

Allow to set undefined properties in yii\base\Object #15845

Closed
mikehaertl opened this issue Mar 7, 2018 · 8 comments
Closed

Allow to set undefined properties in yii\base\Object #15845

mikehaertl opened this issue Mar 7, 2018 · 8 comments

Comments

@mikehaertl
Copy link
Contributor

I have to come back to my closed issue #12021. It seems that this is still not fixed in Yii 2.1.

My description and use case there is still valid.

The outcome of #12021 was, that this should be implemented via a trait. But it seems this never happened in 2.1.

tl;dr: I want to use the magic getter/setter methods from yii\base\BaseObject to be available in my custom classes. But I don't want that this changes PHP's default behavior, where you can also set undefined properties on a class.

<?php
class X
{
}

$x = new X;
$x->demo = 'x';   // No error, PHP allows to have dynamic properties

echo $x->wtf;  // Error, because 'wtf' was not set yet.

Yii changes this default behavior of PHP:

<?php
class X extends yii\base\BaseObject
{
}

$x = new X;

// This will throw an error:
$x->demo = 'x';
@beroso
Copy link
Contributor

beroso commented Mar 7, 2018

You can create your custom Object class that extends yii\base\Object and overrides the __set with your proposed code:

    public function __set($name, $value)
    {
        $setter = 'set' . $name;
        if (method_exists($this, $setter)) {
            $this->$setter($value);
        } elseif (method_exists($this, 'get' . $name)) {
            throw new InvalidCallException('Setting read-only property: ' . get_class($this) . '::' . $name);
        } else {
            // Instead of throwing exception:
            $this->$name = $value;
        }
    }

And your custom classes can extends your custom Object.

@samdark samdark added this to the 2.1.0 milestone Mar 7, 2018
@samdark
Copy link
Member

samdark commented Mar 7, 2018

OK. Let's have another look at it. Yii indeed enhances default PHP objects behavior adding several features:

  • Getters/setters.
  • Strict errors when non-existing property is set.

On top if it, Component adds:

  • Events.
  • Behaviors.

Ideally we want to divide these into traits and be able to use any combination of these traits for any object.

The problem is that same __get(), __set() and other magic methods are used by all these features therefore I don't immediately see a good way to achieve that.

@samdark samdark self-assigned this Mar 7, 2018
@samdark
Copy link
Member

samdark commented Mar 7, 2018

We could probably add a specialized trait that could be configured with an array of handlers for the purpose but I do not see any way for it to be as performant as current solution.

@mikehaertl
Copy link
Contributor Author

I don't know a perfect way either. Each thinkable solution has shortcomings. Still, my current favourite is having a (optional!!) class property like:

public $__strictGetSet = false;

Only if defined this would disable checks for existing properties. The default would be the same behavior we have now.

@mikehaertl
Copy link
Contributor Author

You can create your custom Object class that extends yii\base\Object and overrides the __set with your proposed code:

@berosoboy Sure, but I may also want to extend a more complex Yii class and still disable that behavior. With your suggestion I'd basically end up duplicating a lot of Yii classes in that case.

@SilverFire
Copy link
Member

I'd rather see it as LooseGetSetInterface that may be applied to class to indicate that __get()/__set() methods should not check property existence.

@mikehaertl
Copy link
Contributor Author

@SilverFire You mean an empty interface (interface LooseGetSetInterface {}) that's only used to flag certain classes? I like that idea. It's also straightforward to check for that in the magic method.

mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
mikehaertl added a commit to mikehaertl/yii2 that referenced this issue Mar 8, 2018
@ghost ghost deleted a comment from samdark Oct 9, 2018
@ghost
Copy link

ghost commented Oct 9, 2018

This issue was moved by samdark to yiisoft/yii-core#41.

@ghost ghost closed this as completed Oct 9, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants