Recover the magic abilities of Yii #7

Closed
Sammaye opened this Issue Mar 12, 2013 · 7 comments

Projects

None yet

2 participants

@Sammaye
Owner
Sammaye commented Mar 12, 2013

This issue: #6 ( as provided by @f10i ) shows how you can use try and catch to actually allow Yiis native behaviour with __get and __set to be used.

This might apply to __isset() and __unset() as well.

Edit: no, only the __get and __set since those are the two that override Yiis native abilities.

@Sammaye Sammaye referenced this issue Mar 12, 2013
Closed

Behavior Support #6

@Sammaye
Owner
Sammaye commented Mar 12, 2013

This should now be in commit: 60e8476

It has passed unit testing and am awaiting for confirmation on successful usage in target envo.

@Sammaye
Owner
Sammaye commented Mar 12, 2013

This seems to be working on the target envo, closing as done

@Sammaye Sammaye closed this Mar 12, 2013
@rawtaz
rawtaz commented Mar 12, 2013

Hey guys,

I hate to be picky, but I'm not sure using try/catch for this is very clean. Exceptions are to be just that, exceptions to the norm, rather than something you catch just to see/try if there's a property defined somewhere. This probably aught to be done in a more controlled way.

May I suggest that instead of using try/catch you do what Yii does in its CModel::__get(), see https://github.com/yiisoft/yii/blob/1.1.13/framework/base/CComponent.php#L120 ?

@Sammaye
Owner
Sammaye commented Mar 12, 2013

Due to the way that PHP OO accession of private variables work this means that the code would need to dierctly, in the EMongoModel class, duplicate all variables and functions of CModel and CComponenet and a few other things.

I suppose the exception of the __get is that the variable is not set in the model, if it is not set then our version of norm is to return null instead of throw the exception to the user.

This is the problem with Yii atm, in its current form, the variables are impossible to get at nicely without pretty much coding your own backbone for everything. I could duplicate the code but then if Yii fixes a bug in their code we would be forced to reflect that change in our code base for MongoYii each time.

This is also why I only duplicated a few error functions to support the subdocument validator, I sensed there would not be many, if any, bug fixes required for those very small functions.

@Sammaye
Owner
Sammaye commented Mar 12, 2013

I must admit I am unsure about the changes to the __set, they could be unperformant plus the parent __set throws two exceptions, one for read only and one for non-existant; hmmm

@rawtaz
rawtaz commented Mar 14, 2013

OK, I see the issue now. I had not fully looked into this before writing, my apologies for that.

Right, so calling parent::__get() is fine, but the issue is that it will give an exception if the corresponding attribute doesn't exist. Normally a situation like that is a bug/shouldn't happen, but in this case it is expected to be possible.

I don't see a better way to do this without having to refactor/duplicate other code, so yeah.. Would be useful if the exception raised was of its own type though, to be more specific in the catch.

@Sammaye
Owner
Sammaye commented Mar 14, 2013

Agreed :)

Same for the set, would be good if it threw one type of read only exception and one type for non-existant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment