Arbit - project tracking

Dwoo

#65: objects and virtual properties

Issue revisions

  • new by emanuel at 2010-M-22 13:13
  • closed by Jordi Boggiano at 2010-M-08 18:37
Type bug bug
State closed closed
Priority high high
Resolution fixed fixed
Assigned to Nobody
Scheduled for 1.2.0
Affected versions 1.1.1
Affected components
Last change Saturday 8 May 2010 18:37:58 UTC by Jordi Boggiano

Short description

I have an object like the one from http://www.php.net/manual/en/language.oop5.overloading.php

and I have the following code in the template

{if isset($obj->someVirtualPropertyThatdoNotExists)}aaaa{/if}

I get a notice from the __get function Notice: Undefined property via __get(someVirtualPropertyThatdoNotExists): in C:phpincludesDwooDwoo.php on line 1 in D:...myClass.php on line 47

why the __get function is executed when i do isset!!!!

I see http://bugs.dwoo.org/dwoo/bugs/issue/36 ... I think that we should have a config setting for this... this is breaking correct implemented objects.

Environment

ALL

  • Jordi Boggiano at Monday 22 March 2010 14:30:58 UTC

    I see your point.. I'm not too sure how to address it though.

    The easiest for now is that you add an isset() check in your __get implementation obviously, but I don't know if that is feasible.

    Apart from that, an option sure is possible, but it sucks a bit. I would like to default to the right way since as you mention the current way is breaking correct objects, but that also means BC break.

  • emanuel at Tuesday 23 March 2010 07:51:01 UTC

    I know that an option sucks but I prefer this way then having to remove the notice from my class. Is bad that you have to do the check for __get, they should implement the class correctly in the first place.

  • Sven Rautenberg at Tuesday 23 March 2010 10:40:23 UTC

    emanuel, I doubt Dwoo should act on this issue. Fix your class instead. :)

If you use __get(), then you most likely also have to use __isset().

Demo Code:

<?php

class issetcheck
{
  public function __get($val) {
    echo "Get '$val'\n";
    if ($val == "bar") { return "BAR"; }
  }

  public function __isset($val) {
    echo "Isset '$val'\n";
  }
}

$i = new issetcheck();

echo "Test foo: \n";
var_dump(isset($i->foo));
var_dump($i->foo);

echo "Test bar: \n";
var_dump(isset($i->bar));
var_dump($i->bar);
?>

Demo Output:

Test foo:
Isset 'foo'
bool(false)
Get 'foo'
NULL
Test bar:
Isset 'bar'
bool(false)
Get 'bar'
string(3) "BAR"


There ist absolutely nothing Dwoo or any other class can do to check if a nonexistant property exists in your class. __get() could contain any thinkable complexity to check if a property really exists and which value to return. Even a simple 1:1 mapping to the value of a data array is too complex to be analyzed. It is the task of your class to also define the proper __isset() method to enable checking if a property really exists.

Apart from that: Personally I'd stay away from using the magic methods because they tend to make your code untestable, which is a bad thing.

  • Jordi Boggiano at Tuesday 23 March 2010 10:53:30 UTC

    The thing is, he implemented it right, but dwoo contains a work around because I was too nice to someone with an improper implementation (see #36) :)

    Therefore adding an option to support improper implementations and reverting the default to proper implementation support would be a good idea I guess.

  • Sven Rautenberg at Tuesday 23 March 2010 11:02:13 UTC

    Ok, I don't like to revert my comment, but I get an understanding of the problem now. :)

    Essentially the Changeset 290 is partly wrong. Dwoo checks if a property isset(), which implicitly would call __isset().

    If this magic method returns false, then the object does not have this property and should not be bothered getting it, but Dwoo ignores the __isset() result if __get() is implemented.

    That should only be done if __isset() is NOT implemented. Which really complicates things. Dwoo would try to fix things inside objects which act as data source, and that is way beyond the scope of a template engine. The data objects given to the template should act appropriately, or the user must live with the consequences.

    I'd vote on reverting r290.

  • Jordi Boggiano at Saturday 8 May 2010 18:37:58 UTC

    Fixed in git 90ad6a8097c55f16459676e44de0a89109cc45d5, I reverted the patch from #36, the change will be replicated to svn trunk within a few minutes.

  • Jordi Boggiano at Saturday 17 July 2010 17:40:53 UTC

    Eh sorry I don't know what the fuck I did, but seems I forgot to push it, but I've finally done that http://github.com/Seldaek/Dwoo/commit/71c1d67c8fa66a99b54584a2341f1198595d3c49

Arbit - project tracking, licensed under GPL 3