- I'm moving this into its own mail thread because talking about 3* different topics under the same chain is ridiculous (has anyone suggested forums instead of email??)
So here comes my round of feedback on the current proposal.
But before getting to that: I have collected a bit of data how getter and setter are currently used in Symfony and ZF:
https://gist.github.com/3884203 Mainly so I get a better overview of the situation, but might be of interest for other people involved
in this discussion too.
So, my points on the proposal, in no particular order:
- Protected backing property
The RFC currently contains just a single code example, namely setting a $Seconds property through an $Hours property, which is an
example where you want to override both accessors and there is a backing property that already exists "naturally".
If you on the other hand you consider cases where you only want to override either the set or the get behavior and there doesn't
already exist a backing property, you will end up with a code like this:
protected $_isEnabled;
public $isEnabled {
get() { return $this->_isEnabled; }
set($isEnabled) { $this->_isEnabled = (bool) $isEnabled; }
}
The above code really only adds a bool cast, but still it has to do two things: Add a protected backing property and define a getter
which basically matches the default behavior. With automatic properties as they are currently implemented you could write:
public $isEnabled {
get();
set($isEnabled) { $this->__isEnabled = (bool) $isEnabled; }
}
This is closer to what I actually want to do, but it relies on using $this->__isEnabled, which is really just an implementation detail.
Instead, I would rather want to have access to the "underlying" property:
public $isEnabled {
get();
set($isEnabled) { $this->isEnabled = (bool) $isEnabled; }
}
I know that the wording is a bit vague; another way to put it, is that the above inverts the current shadowing behavior:
In the current implementation, if there exists a "plain" property and an "accessor" property with the same name, then the plain
property shadows the accessor property. As you say, this is to help lazy-loading. I don't really buy the argument about lazy-loading,
because I don't think that the small perf difference between going trough an accessor and accessing a plain property is really relevant
in that context (especially as lazy-loading would have required a method otherwise).
What I'd rather see is the reverse behavior: An accessor property shadows a plain property, so that the plain property is only
accessible from within the accessor methods. This would allow you to write code like in the last example above and I think it would
also make automatic properties more meaningful (you could store the state in a property with the same name; also nicely integrating
with serialization and debugging).
I know that you (Clint) want to create a hard distinction between plain properties and accessor properties, but I think that making it
more smooth juts integrated better with PHP. I already noticed several people who, after reading the RFC, tried to write code where
they access the property from within the accessor. It seems to me that people think it should work. The current behavior where you
can shadow the accessor property by assigning a property of the same name seems rather obscure to me.
This is the feedback I have for now. Looking forward to your thoughts on the manner.
Nikita
I would be interested to see if you could change your analysis code to detect "lazy loading" type use. My own use of __get() is primarily made up of lazy loading, followed by filtered access (eg: tblEnabledFilters), followed by aliases to sub-object properties.
But in the end I can agree with all of what you've wrote here because lazy loading could still be implemented as such:
public $xyz {
get() { return $this->xyz = $this->xyz ?: /* Do some loading */ }
protected set($x) { $this->xyz = $x; }
}
But here are some issues with the above that may be of concern:
-
Property access is 4x faster than accessor method calling
-
This would (without a big change in the current implementation) a secondary lookup against accessors for every property access
o The way around this issue is to merge accessors and properties such that the same properties struct that exists now would contain all of the new properties (or at least a pointer to such.) This may indeed be a much better way to go anyways.
One additional benefit of keeping any backing-field be the same name as the accessor is that you can more easily mix automatic implementation with non-automatic, for example the above could simply be left as:
public $xyz {
get() { return $this->xyz = $this->xyz ?: /* Do some loading */ }
protected set($x);
}
But here are some issues with the above that may be of concern:
Property access is 4x faster than accessor method calling
4x faster on what kind of scale? Where are these performance tests at?
master
Cycles Direct Getter __get
v1.4 @ 10/8/2012 1m .05s .21s .20s
php 5.5.0-dev
Cycles Direct Getter __get
v1.4 @ 10/8/2012 1m .04s n/a .21s
"Direct" vs "Getter" vs "__get" on 1 million cycles.
-----Original Message-----
From: Levi Morrison [mailto:morrison.levi@gmail.com]
Sent: Monday, October 15, 2012 9:16 AM
To: Clint Priest
Cc: Nikita Popov (nikita.ppv@gmail.com); internals@lists.php.net
Subject: Re: [PHP-DEV] [PHP-DEV [RFC] Property Accessors v1.2 : Backing PropertyBut here are some issues with the above that may be of concern:
Property access is 4x faster than accessor method calling
4x faster on what kind of scale? Where are these performance tests at?