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 #12021

Closed
mikehaertl opened this issue Jul 26, 2016 · 4 comments
Closed

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

mikehaertl opened this issue Jul 26, 2016 · 4 comments

Comments

@mikehaertl
Copy link
Contributor

mikehaertl commented Jul 26, 2016

For 2.1 I'd like to propose a change in yii\base\Object to make it behave like standard PHP objects again, when it comes to setting undefined properties.

This change could also be configurable, if some consider it to be too drastical.

What is it about?

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

$x = new X;

// This will throw an error:
$x->demo = 'x';

You will see an yii\base\UnknownPropertyException.

How does PHP behave usually in this case?

Any class in PHP allows to set undefined properties, just like stdClass. So if you take the above example but don't extend from yii\base\Object, it will work:

<?php
class X
{
}

$x = new X;
$x->demo = 'x';   // No error

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

All Yii2 objects break this standard behavior, so you lose a feature that works for all other PHP classes.

Why does this matter?

Say you have a complex SOAP service with 100s of SOAP types, some nested into each other. For example there could be a User type object somewhere in the SOAP result. By default PHP will return a instance of stdClass with all the properties set. Something like this:

 stdClass Object
(
    [id] => 123456,
    [firstName] => 'John',
    [lastName] => 'Donson'
    [street] => '...'
...

Now you wish to have a fullName property. You therefore could define a custom class with a getter method like:

namespace app\models\soap;
class User
{
    public function getFullName()
    {
        return $this->firstName.' '.$this->lastName;
    ]
}

And configure it in the classmap option of your SoapClient:

'classmap' => [
    'User' => 'app\models\soap\User',
],

This works, but you still have to call $user->getFullName(). If you like to do it the Yii way and have access to $user->firstName you can't, because your custom class can not extend from yii\base\Object. You would have to define each and every property of the User SOAP type manually - something you don't have to do in pure PHP classes or in stdClass in this situation.

Proposed solution for Yii 2.1

Instead of throwing an error in Object::__set() for undefined properties, it should simply set the property:

    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;
        }
    }

I'm not quite sure what would be a good way to make this optional. Object should stay free of any properties as it's a base class. And defining another method like __allowUndefined() seems like overkill. Maybe a property like protected $__allowSetUndefined = false;?

        } elseif ($this->__allowSetUndefined) {
            $this->$name = $value;
        } else {
            throw new UnknownPropertyException('Setting unknown property: ' . get_class($this) . '::' . $name);
        }
@samdark samdark added this to the 2.1.x milestone Jul 26, 2016
@samdark
Copy link
Member

samdark commented Jul 26, 2016

The behavior of being strict about setting undefined properties could be moved into a trait same as ability to access getters as properties.

@mdmunir
Copy link
Contributor

mdmunir commented Jul 27, 2016

@cebe
Copy link
Member

cebe commented Aug 2, 2016

Not allowing to set unexisting properties is a feature I would keep tied to the configuration feature of yii as it helps prevent typos in configuration files.
Otherwise you could add non existing properties in configuration and not notice that you have a typo wondering why it has no effect.

We could split the features into traits so one can implement what is needed dependent of the purpose of the class.

@SilverFire
Copy link
Member

Agreed with @cebe

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

5 participants