Speaking from my experience, I naturally prefer code that prevents
problems, and I prefer code with more checks instead of fewer checks,
because I've seen so much "abusive" PHP code with inadequate error
checking that required fixing later ;)
For even the extreme PHP web applications supporting thousands of
simultaneous users, I've found these groups of developers usually gladly
shed a few microseconds of CPU time on their web servers for improved
reliability and error checking to reduce future maintenance effort.
Actually, I've seen them do remarkably CPU wasteful things and not care,
since they usually only need to tune the architecture to minimize DB
connections and usage.
Thus, I favor making the magic methods protected, since I have not yet
seen a use case justifying making them public, and the team previously
opted for fewer checks in the magic methods.
2 cents,
Gavin
Ralph Schindler wrote:
Hi all,
I'll do my best to explain why this even came up... where it went,
and how we got here. Originally, the function existed as such:
public function __set($name, $value)
{
if (!is_string($name) && $name !== '') {
throw new Zend_Session_Exception('Z_S expects a string');
}
}
To me, this seemed the absolute best approach assuming a: never trust
all php developers to do the 'right' thing when the fact of the matter
is php in alot of cases is an entry point to programming. Don't ask
me to produce evidence of this, this is why php is wildly popular, and
this is also why php5 adoption is staggering. AND b: this is how the
docs have it, it is known to work like this, and sensibly (which
probably could be substituted with 'luckily') this is the way things
will work in future versions of php.
After a code review, zend'ers decided
(http://framework.zend.com/fisheye/changelog/Zend_Framework?cs=1638)
it would be best to save a few ticks of processing and to remove the
is_string() check. I am ok with this move, I am all about speed. But
what you gain in speed, you lose in clarity since there are several
vectors which I have seen developers use that will certainly break any
app and NOT provide sufficient error reporting, thus, developers will
waste time wondering why their variable variable call to the session
object is not working as expected when somehow an array got passed to
one of the magic functions and things are now either a) not working as
suspected and not throwing notices (b/c honestly, people don't code
with E_STRICT) or b) someone is calling a magic method directly b/c
they are simply now PHP OOP aware (and this is real COMMON.. go to a
php meetup group if you want proof).
To me, is_string() is the 80/20 rule. It locks down the api to a
sufficient level that will reject all obvious wrong usage.. B/C IMO,
if you give someone enough rope, a ladder, and a high beam.. with a
framework that doesn't present the correct error reporting.. they
WILL hang themselves.
Since, I am pretty easy going in general, I went with zend on their
decision and suggested that if thats the case, the best thing to do
since the best solution was not possible, is to go with the second
best solution. Change it to protected. I don't like guesswork at the
core level and to me, removing is_string() and making these public is
adding a common misuse vector that will produce insufficient error
reporting.
This is where we are now, discoursing over the addition of an error
checking function vs speed. Hopefully at the very least, the
conclusion of this will at least provide us with coding standard.
In the end, I don't care so much what the group decides, b/c frankly
I've intimately familiar with PHP and PHP OOP5, and I wouldnt misuse
this in such a way.. I'm just trying to stick up for the new guy,
just learning php and decided to start with the brand spaking new php
framework from zend.
</stepping down from soapbox now>
ralph
