Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:64489
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zerocue.com designates 67.200.53.250 as permitted sender)
Message-ID: <50E4BDDE.8050509@zerocue.com>
Date: Wed, 02 Jan 2013 17:08:14 -0600
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Thunderbird/17.0
MIME-Version: 1.0
To: Steve Clay <steve@mrclay.org>
CC: PHP Internals <internals@lists.php.net>
References: <50E41BB6.4030901@zerocue.com> <CADEasVEHFOgC_f-YFHPu4JoWogLgjiLouNX55oQUgSDyLM_Ggg@mail.gmail.com> <50E4A43E.6030302@zerocue.com> <50E4B232.5000505@mrclay.org>
In-Reply-To: <50E4B232.5000505@mrclay.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PHP-RFC] Property Accessors 1.2 for Final Review before
 Vote
From: cpriest@zerocue.com (Clint Priest)


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