On Tue, 28 Jun 2011 18:44:49 +0100, Terin Stock <terinjo...@gmail.com> wrote:

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 to isset().

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 important.

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 that.

You're of course free to use modified version of PHPTAL if you wish.

regards, Kornel Lesiński

PHPTAL mailing list

Reply via email to