[PHPTAL] [PATCH] Shouldn't skip over __get() just because __isset() was defined…
kornel at geekhood.net
Tue Jun 28 20:33:09 CEST 2011
On Tue, 28 Jun 2011 18:44:49 +0100, Terin Stock <terinjokes at gmail.com>
> isset() isn't defined for lazy-loading properties, they're either set or
> not set.
IMHO lazy-loading is an implementation detail that should not be visible
The state you have is "property is there and it has a value, but the value
is still in the database", but your __isset() says "property is not there
or has no value", which is a different thing.
> Not "not currently set, but could be in the future". Therefore, is it
> correct or incorrect to define it one way or another in our
> application's objects?
If you're saying that property is not set (yet), then PHPTAL will not use
it (yet). I don't think it's safe for PHPTAL to assume that every unset
property will appear when __get() is used.
> In the current implementation, PHPTAL makes an active attempt at
> stopping me from having __isset defined in a way logical for the rest of
> my application.
It may be convenient for your application, but I don't think that's good
solution for all applications that use PHPTAL. I believe consistency with
isset() and ability to avoid double-checking with __get() is more
I think your application should use different method for exposing
lazy-loading state (e.g. isLoaded($property)) and use __isset() for
reporting existence of properties whether they're loaded or not.
> My patch fixes that, and doesn't change anything if you have decided to
> implement __isset the other way.
Thank you for your patch. It's not fun to reject patches, I'm sorry for
You're of course free to use modified version of PHPTAL if you wish.
regards, Kornel Lesiński
More information about the PHPTAL