#2: I agree with you here - this is problematic. isset/unset accessors that invoke the isset/unset function should actually invoke the function rather than being compared to null, as that isn't the same as isset.
#5: From what I understand, an extending class can not override an accessor with a non-accessor. #11: If you set an accessor's get or set to *final private*, you are not able to extend and it are only able to invoke it from the current class. If you don't invoke it, then it is virtually read or write only. On Tue, Oct 16, 2012 at 2:12 AM, Stas Malyshev <smalys...@sugarcrm.com>wrote: > Hi! > > > https://wiki.php.net/rfc/propertygetsetsyntax-as-implemented > > My feedback on the RFC: > > 1. Accessors IMO should be regular PHP methods that PHP generates with > two additional things: > a. Their name is generated by PHP > b. Their argument set is defined by the accessor pattern (i.e. same > thing as __get/__set). > We should keep the amount of magic and special cases to the minimum, the > engine is complex enough as it is. > This of course includes the full range of options available for the > methods - final, reflection, call scenarios, etc. > > 2. isset is defined as: > isset() { return $this->Hours != NULL; } > This does not seem to be correct - != NULL does not work like isset now. > Try this: > class A { public $x = 0; } > $a = new A; > var_dump(isset($a->x)); > var_dump($a->x != NULL); > This needs to be fixed - generated isset() should be defined to work > exactly like regular isset and actually use the same code path. Argument > to isset() call can be retrieved using accessor but it should not differ > from isset() result in any possible way or situation (including error > situations - e.g. notices, etc.) > > 3. How references and complex cases are handled? Didn't find anything > about it, e.g. how it handles $foo->bar++, $foo->bar[] = 1, > $foo->bar[123] = 4, etc. ($foo->bar being property with get/set defined > of course)? How $foobar =& $foo->bar is handled? The sort() case > mentions by-ref return but does not explicitly mention all other cases. > These need to be covered explicitly in the RFC. > > 4. We have some LSP controls now in place to ensure non-LSP overrides > generate E_STRICT. Will this be the case for properties too? Meaning, > what happens if you add overriding protected getter to a property that > was previously fully public - thus violating the LSP? > > 5. What happens if you override accessor property with plain old > variable property? I.e.: > > class A { > protected $secret { > get() { return "secret"; } > } > } > > class B extends A { > public $secret = "not a secret anymore"; > } > > Also, what happens here if A extends B in the same scenario (this refers > to the #4 too). > > 6. Thinking more about isset/unset - why not make isset/unset always be > the default unless overridden? I can't really see the case where you > want echo $foo->bar to work but if(isset($foo->bar)) echo $foo->bar to > not work. I think isset() and unset() should always use automatic > implementations by default (fixed in accord with #2 of course). > > 7. "Error messaging" section is not clear. Some examples would help - > what is being translated to what? > > 8. "Static accessors" section is not clear, namely this one: > This yielded the possibility that a getter call was being made while it > should not be allowed (if there was no getter defined) and so pass_two() > was changed to look for these non-backpatched illegal static getter > calls and a compile time error is produced. > > When the code is compiled, the class definition (and, in fact, the name > of the class we're talking about) may not be available, so how can you > known if certain property of this class has getters defined? > > Also, I'm not sure what is the thing about backpatching and converting > to function calls - I think it should work via engine handlers just as > the rest of the things work in the engine, is it not the case? If not, > it should be made the case. > > 9. This: > Eliminate the ability for an accessor to be called via $o→__getHours(), > the accessor functions will be completely unavailable for use except as > property references ($o→Hours) > > I think it a mistake. More magic and complication in the engine is not a > good thing and would require tons of special case checks in all places > where we deal with functions. I think it is wrong - if we create a > callable entity, it should be callable. __ in the name is the indication > enough that you're dealing with special method and you should tread > carefully, we should not go out of our way to mess up the engine to > prevent it. > > 10. I'm not sure what the point about debug_backtrace() is but again, we > should not create more complicated magic there, it just should show what > really is happening. > > 11. About read-only/write-only thing - I like not having the keywords, > but it is still not clear how final produces read-only property - do I > say something like "final set();" or how can I say "there's no set and > never will be"? > > I'm sorry if some points were already discussed - I scanned through the > multiple threads but may very well missed something in 100+ messages > that there are, in any case I feel most of the questions above must be > reflected in RFC explicitly. > > Thanks, > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
