Re: Property Accessors 1.2 for Final Review before Vote

On 1/2/2013 4:18 PM, Steve Clay wrote:

On 1/2/13 4:18 PM, Clint Priest wrote:

Omitting isset/unset has the same effect as declaring it without a body. This is described in the RFC under Automatic Implementations with this line: Note that isset/unset implementations will always be provided if they are not defined or if they are explicitly auto-defined (as above).

I think the RFC could make this clearer: "isset & unset are always provided with the default implementations unless the author provides his/her own." I can do that, no problem.

Looking closer at the default implementations of isset/unset, I'm worried these could lead to confusion. Consider this code: class TimePeriod {

    private $Seconds = 3600;
    public $Hours {
        get { return $this->Seconds / 3600; }
        set { $this->Seconds = $value; }
    }

} The RFC's default implementation is always bound to the shadow property, so here's what you really get: class TimePeriod {

    private $Seconds = 3600;
    public $Hours {
        get { return $this->Seconds / 3600; }
        set { $this->Seconds = $value; }

        // auto-generated
        isset { return $this->Hours != NULL; }
        unset { $this->Hours = NULL; }
    }

} Note the resulting behavior: $t = new TimePeriod; $t->Hours; // 1 isset($t->Hours); // false !? unset($t->Hours); $t->Hours; // still 1 Effectively, authors who don't base their getters/setters on the shadowed property must be urged to create their own isset/unset because the default ones would be useless. I'm not crazy about this. Sorry, there was a typo in that RFC there, this line:

        isset { return $this->Hours != NULL; }

Should have been with !==:

        isset { return $this->Hours !== NULL; }

I've already updated the 1.2 doc to reflect the correct way. Given what I mentioned above, I'm assuming you did not test this with the fork, right? Just based your comments on how it should logically work (with the incorrect != vs !==?) One last thing about that, the isset/unset with $this->Hours calls the getter to retrieve the $this->Hours value, so it behaves as your example below indicates.

I'd prefer these default implementations: isset { return $this->__getHours() != NULL; } unset { $this->__setHours(NULL); } $t = new TimePeriod; $t->Hours; // 1 isset($t->Hours); // true unset($t->Hours); $t->Hours; // null isset($t->Hours); // false Note these also work as expected when using the default get/set implementations. Of, course, my implementations don't actually *work* because you can't call an accessor from an accessor... Steve, are you testing these w/ the fork? Sounds like you are... But your above sentence is not accurate, you can call an accessor from an accessor. isset { return $this->Hours !== NULL; } calls the getter for the value and compares it with NULL.

Steve Clay

-- -Clint