I think this is a great case study that could lead to some improvements in the process of fixing bugs in PHP:
http://bugs.php.net/bug.php?id=45743
A potential bug was reported on property_exists() that it won't report inaccessible properties in the scope.
The problem is this was, in fact, the intended behavior, and despite everything saying the function works correctly, everything around it was changed in order to agree with the new behavior for no good reason that I can see:
-
A test existed that specifically tested propert_exists() respects visibility of the property from the call scope. So for this 'bugfix' to pass tests, the test was simply changed.
-
The documentation specifically said the function respects visibility of the property, so for this 'bugfix' to agree with documentation, the documentation was changed.
-
The use case for this function required the existing behavior (i.e. can you safely call property $x from the current scope with no warning?), and that ignored when the behavior was changed. We already have Reflection classes for inquiring protected/private properties of a class from any scope.
So, what happened here?
Stan Vass
Hi!
- The use case for this function required the existing behavior
(i.e. can you safely call property $x from the current scope with no
warning?), and that ignored when the behavior was changed. We
already have Reflection classes for inquiring protected/private
properties of a class from any scope.
Maybe we can fix this by having an argument to the function that if
false, checks only for existence, and if true, checks also for access?
On the other hand, we already have isset() for vars and is_callable()
for methods, so this use case might to be covered.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
- The use case for this function required the existing behavior
(i.e. can you safely call property $x from the current scope with no
warning?), and that ignored when the behavior was changed. We
already have Reflection classes for inquiring protected/private
properties of a class from any scope.Maybe we can fix this by having an argument to the function that if
false, checks only for existence, and if true, checks also for access?On the other hand, we already have isset() for vars and
is_callable()
for methods, so this use case might to be covered.Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
A flag seems like a viable solution, but I'm worried that something that was
flagged as a "bug", was not, and caused change in docs, tests and behavior,
while all agreed with each other before.
This is at beast a "feature request" which should have been discussed as
such, considering BC break implications and so forth.
As for isset(), it doesn't cover the use case for property_exists(), as
isset() returns false when a property exists, and is one of these values:
null, false, int 0, float 0, string '', empty array()
property_exists() more closely relates to "array_key_exists() for objects"
in that regard.
Stan Vass
Hi!
As for isset(), it doesn't cover the use case for
property_exists(), as
isset() returns false when a property exists, and is one of these values:null, false, int 0, float 0, string '', empty array()
property_exists()more closely relates to "array_key_exists() for objects"
in that regard.
This is not true. isset() only returns false if the property doesn't
exist or is set to null.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
2010/10/13 Stas Malyshev smalyshev@sugarcrm.com
Hi!
As for isset(), it doesn't cover the use case for
property_exists(), asisset() returns false when a property exists, and is one of these values:
null, false, int 0, float 0, string '', empty array()
property_exists()more closely relates to "array_key_exists() for objects"
in that regard.This is not true. isset() only returns false if the property doesn't exist
or is set to null.
yeah, but property_exists would return true for null value if the property
itself exists, so they don't 100% interchangeable
Tyrael