I have some observations about the method `fireValueChangedEvent` that most PropertyBase classes provide.

They seem to have several design problems:

1) It's specified that it is called internally when `set` is called (dangerous, as it can be overriden and not do anything breaking the contract of the PropertyBase classes).

2) When overridden, it does the exact same thing as `invalidated` (they're always called at the exact same time) -- perhaps `invalidated` was a later addition?

3) When called externally, it can trigger invalidation listeners over and over; there is no check if the property was actually valid.  It just fires all listeners ignoring the normal constraints.  Notifying invalidation listeners again when already invalid is the use case they were designed to avoid.

My problem with this implementation is primarily in the fact that the implementations of the PropertyBase classes are **forced** to call it internally, as otherwise classes which override this method would "miss" this "event".  But that isn't the only issue; I could just leave the implementation empty and call both that method and another method that does what I want, but that runs into a different problem: if I leave the implementation empty, then legitimate external calls of `fireValueChangedEvent` would become no-ops.

So, I can't change the implementation because then it doesn't do anymore what external parties expect, and I can't **not** call it because then overriders wouldn't be informed of the change.

The reason this is bothering me is because I see a way to optimize memory use for all properties that have a change listener attached, but I can't make the change because the Base classes have exposed some unfortunate internals.  The optimization is simple; a property has its value, but so does ExpressionHelper, it has a copy -- they both have the "current" value of the property. It would be trivially easy to supply the ExpressionHelper with the old value by saving this value off just before changing the property's value, and then supplying it when calling `ExpressionHelper#fireValueChangedEvent`... except for this one little snag where I'm forced to call `ObjectPropertyBase#fireValueChangedEvent` of which I can't change the implementation...

Unfortunately, I don't think there is a way to solve this, so we're stuck with having to store the current value of a property in two places.  In theory, the `fireValueChangedEvent` method could be made final, and any code that overrides it could override `invalidated` instead.  In that case, internal calls could go via a different route allowing for supplying an old value when its value was set directly, while external callers can still use the existing method (which will use the current value as its old value, and preferably, also avoids calling invalidation listeners when not needed).

--John

Reply via email to