25

I have an abstract parent class Mongo_Document (from mongodb-php-odm) and an inherited class Model_ActionPlan. Mongo_Document has magic __isset and __get methods that interact with an array inside the Mongo_Document class.

I am trying to use the following code (snippet from inside a method of Model_ActionPlan):

if (isset($this->status))
{
    if (($this->status === "closed") AND ($this->close_type != "failure"))
    {
        return;
    }
}

(Note that close_type is guaranteed to be set if status == 'closed'.)

The isset call returns true and then execution proceeds to the next statement. There, I receive the following error:

 Undefined property: Model_ActionPlan::$status

However, if I replace $this->status with parent::__get('status'), this code works as expected. Note that everywhere else in the program, I am able to use:

$ap = new Model_ActionPlan($plan_id);
echo $ap->status;
// Prints 'closed' (or 'active') as expected

It is only here, inside the class itself, that this doesn't work.

I looked around and I can't seem to find anywhere that says that magic methods can't be called in the child class. I could use the parent::__get call instead but I think that is probably the wrong way to do it. Does anyone know if there is a right/better way to do this?

Updated #1 2012-12-16: The full code of the parent class is here on Github.

Updated #2 2012-12-18: For the people who asked about where or whether it is set properly, the answer is that since calling parent::__get('status') does work, the problem is obviously not that the variable isn't getting set. The __get() is getting its data from a private instace variable called _object. If I var_dump($this), I see that $this->_object['status'] does equal the expected "closed" value.

Update #3: The code of the child class is available at https://gist.github.com/4332062. The important part starts on line 69.

I have seen this similar question but that one is about using a parent's magic method to get the child's properties and my issue is using the parent's getter to get the parent's properties.

Community
  • 1
  • 1
Moshe Katz
  • 15,992
  • 7
  • 69
  • 116
  • What version of PHP are you on? This is not supposed to happen. – Ja͢ck Dec 11 '12 at 03:09
  • I am using `5.4.8-NTS-VC9 (Windows FastCGI)` on my local machine and `5.3.10-1ubuntu3.4` on my testing server. The problem occurs on both. – Moshe Katz Dec 11 '12 at 04:13
  • I've tried to reproduce the issue by creating a base class with magic methods and then extend it, but that doesn't seem to exhibit the issue that you're experiencing. – Ja͢ck Dec 11 '12 at 04:16
  • 1
    please add more code from parent class where __get and __isset is overloaded. By the way pay attention behaviour of "AND" and "&&" is different – Igor Vizma Dec 16 '12 at 19:51
  • 1
    @IgorVizma I added a link to the source of the parent class on Github. Yes, I know the behavior of `AND` and `&&` is different. That's why there are so many parenthesis in there. Our code style guide says that we should use `AND` and `OR` with extra parens as needed because the person who wrote it thought that would be easier to read. The only difference between them is the precedence, and the parens fix that. – Moshe Katz Dec 16 '12 at 22:21
  • Without more of your `Model_ActionPlan` we can only guess what's going on. – Ja͢ck Dec 18 '12 at 15:21
  • Is 'status' magically set? – Quentin Engles Dec 18 '12 at 17:08
  • @QuentinEngles Yes, status is magically set. It is loaded from a database, and if I `var_dump` the object, you can see that `_object['status']` (the place that the magic method is looking for its data) is set. So the `__isset` method is the correct one and the `__get` is the problem. Note that I mentioned that calling `parent::__get('status')` *does* work. – Moshe Katz Dec 18 '12 at 18:23
  • 4
    Makes no sense. I copied the class, and tried to reproduce it. Nothing went wrong. – Quentin Engles Dec 18 '12 at 21:15
  • You mention that `$this->status` works elsewhere in your subclass...have you tried generating stack traces of uses of `$this->status` to the place where it doesn't work? Perhaps its something to do with the context of your use (ie, some other internal state in Mongo_Document). – dsummersl Dec 20 '12 at 10:57
  • 2
    Unable to reproduce with 5.4.9 & 5.4.10. When you follow that with a step debugger (Xdebug), *what does actually happen*? – hakre Dec 21 '12 at 14:20
  • Here is a stack trace: https://gist.github.com/4353369 The request flow is as follows: the MVC framework is trying to execute `Model_Game->close`. That loads up all of the questions in this game and calls `Model_Question->close` on each one. Each question loads all of the connected Action Plans and calls `Model_ActionPlan->close`. Note that `Model_Question` also has `status` checked - and that one works properly! – Moshe Katz Dec 21 '12 at 15:08
  • Could you check [this code](https://gist.github.com/4372974) on your PHP installation ? – Alex Dec 25 '12 at 12:29
  • @MosheKatz Can you verify that in the `Mongo_Document` class the `$name` in the method `__get` is indeed 'status' (or to be flake, it enters the parents `__get` method at all?) and if so, wether it enters the first `if` statement, or in other words it reaches the end of `__get`. Also does `$this->load()` in the `Mongo_Document::__get` get called when using `$this->status`? – dbf Dec 25 '12 at 13:59
  • @dbf See the stack trace in one of my previous comments. It never calls the `__get` it just throws the error. – Moshe Katz Dec 25 '12 at 16:22

2 Answers2

3

The __get function in the parent is somewhat complex, so I haven't fully figured out whether the following might be happening or not. If your __get function, once called, is in some way triggering another call back into itself (perhaps with some intervening calls to other functions on the stack), this is exactly what would happen.

See http://php.net/manual/en/language.oop5.overloading.php#55486, which shows the exact same type of error being logged when the getter triggers a call to itself. In that instance, it is pretty easy to spot, but with a more complicated call graph of something like func() to __get() to funcB() to funcC() to __get(), it would not be easy to spot.

DWright
  • 9,258
  • 4
  • 36
  • 53
  • You know, you might have something there. I'll have to check if that's what is happening. I don't think the `__get` is calling itself, but it could be that some other kind of "calling itself" exhibits the same behavior. – Moshe Katz Dec 24 '12 at 02:03
  • Unfortunately, I was unable to test whether this was correct before the bounty expired. I'm sorry. However, I do have to say that this is almost definitely the correct answer. I removed one line of code that could cause nested calls to __get and that fixed the problem. – Moshe Katz Dec 26 '12 at 22:32
-3

Not sure if i am right but shouldn't you use 

$this->_object['status']

instead of 

$this->status
Taryn
  • 242,637
  • 56
  • 362
  • 405
mantish
  • 400
  • 3
  • 12
  • That would be true if the magic getter was only getting the array element. However, there are things that the `__get` method does before it gets the element from the array. I need to use the `--get` method in order to have it do that stuff. Additionally, I think it is bad practice to play around with the internals of the parent class instead of using the proper access method if such a method exists. If you use `$this->_object` and then the parent implementation changes, you have to worry about it. If you use the documented methods, then you don't. – Moshe Katz Dec 19 '12 at 20:00
  • Probably more worth a comment as this is more a question than a definitive answer. – Jimbo Mar 27 '14 at 14:34