how to implement JavaFX property "throwing" exception

[Richard Bair]

Hi Vasiliy,

We do have a couple such cases. For some of these what we did was provide the setter and a read only property. I think Stage or Scene has examples of such properties.

Richard

On Apr 5, 2013, at 11:06 AM, Vasiliy Baranov <vasiliy.baranov at oracle.com> wrote:

> Hi Richard,
> 
> Thank you for taking time to explain this. My take away here is, even though some existing JavaFX code throws an exception when a property state is changed, new code should never do so.
> 
> FWIW, the property I have in mind here is the path to the file system location where WebView would persist its local data. (Yes, that is a feature that has yet to be discussed on this list.) There are two facts about that property that I think are relevant for this discussion. First, we really need to throw an exception e.g. when the caller attempts to set the property to a location that is already in use by a WebView running in another JVM; that is because we need to make such an error immediately and explicitly visible to the caller. Second, it looks like no one ever really going to benefit from being able to bind or bind to that property. Taking into account these two things and the recommendation to not throw exceptions from JavaFX property state changers, I tend to believe my property is a bad candidate for a JavaFX property and is better off being a plain old field with a setter method explicitly documented and implemented to throw appropriate exceptions. What do you think?
> 
> Thank you,
> -- Vasiliy
> 
> On 05.04.2013 21:18, Richard Bair wrote:
>> This is a good question. The recommended solution is to avoid the need to throw the exception. We should *never* throw an exception when a property state is changed. We should instead define things such that there is a valid meaning to whatever value the developer has specified.
>> 
>> As you found, in a couple places if the property was bound to another property and, via that binding, we are given an illegal value the only thing you can do to *not* have the illegal value is to unbind the property. That is pure, unadulterated, evil. We should never under any circumstance auto-unbind somebody!
>> 
>> The binding issue is the reason why a property should never throw an exception. The way bindings are implemented is that they are lazily evaluated. But, we can demonstrate easily that when bindings are used, if you throw an exception, then you end up with painful order dependencies in the code. Suppose:
>> 
>> class A {
>>     public final IntegerProperty i = new SimpleIntegerProperty(this, "i", 10);
>> }
>> 
>> A a1 = new A();
>> A a2 = new A();
>> a1.i.bind(a2.i);
>> a2.i.set(100);
>> 
>> In this case, the new value, 100, is not set immediately on a1.i rather, it is only when I *read* from a1 that it would be set. Now if we modified A so that it throws an exception when i is out of bounds, we get really odd behavior:
>> 
>> class A {
>>     public int max = 1000;
>>     public final IntegerProperty i = new SimpleIntegerProperty(this, "i", 10) {
>>         int oldValue;
>>         @Override protected void invalidated() {
>>             if (get() > max) {
>>                 if (isBound()) {
>>                     unbind();
>>                 }
>>                 set(oldValue);
>>                 throw new IllegalStateException();
>>             } else {
>>                 oldValue = get();
>>             }
>>         }
>>     }
>> }
>> 
>> One problem here is that the presence of "get()" just turned this into an immediate evaluation scenario. EVIL! But beyond that, we have totally broken the user from very legitimate use cases.
>> 
>> A a1 = new A();
>> IntegerProperty x = new SimpleIntegerProperty(this, "x", 100);
>> IntegerProperty y = new SimpleIntegerProperty(this, "y", 10);
>> a1.i.bind(x.add(y)); // or some other expression
>> 
>> // then later
>> x.set(1000);
>> y.set(0);
>> 
>> This looks like it should still satisfy the constraint that a1.i be < 1000, but it doesn't. For a brief moment in time, a1.i was > 1000 (it was 1010 to be precise at the time x.set(1000) occurred because the invalidated method forces eager recomputation). Now, the user's a1 is *AUTOMATICALLY UNBOUND* and will forever after SILENTLY NEVER UPDATE AGAIN!!!!
>> 
>> Note also that the developer might not be setting these values in code. Rather, they could be wired up to sliders in a user interface. They'd have to fix up the UI so that the user is prevented from entering an invalid state, or they'd have to proxy the values such that the user can pick any values and they the developer will clean them up before passing them on to us.
>> 
>> Clearly, this is wrong on the mega-atomic scale and should not be reused. So what are the options? The only real option is to not throw an exception. Make the illegal value have a valid meaning. If you have a max, and somebody exceeds the max, what can you do?
>> 
>>     - Have an error event that you raise (but accept the value anyway)
>>     - Have another read-only property that emits the "normalized" value. So although "i" might be > max, "normalizedI" would always fall within the right constraints
>>     - Document that "if i > max, then we will treat it as though i == max"
>> 
>> There are probably other solutions as well. This design problem is a natural outgrowth of the fact that we have lazily evaluated binding (which is required for binding to work in a performance critical manner). If we didn't have lazy binding, then we could have devised a different system.
>> 
>> Richard
>> 
>> On Apr 5, 2013, at 9:33 AM, Vasiliy Baranov <vasiliy.baranov at oracle.com> wrote:
>> 
>>> Greetings,
>>> 
>>> I need to implement a JavaFX property that may only be modified while the object is in an appropriate state, and so that any attempt to modify the property while the object is in an inappropriate state must result in IllegalStateException.
>>> 
>>> My first and naive implementation for this was as follows:
>>> 
>>>    private StringProperty foo;
>>> 
>>>    public final StringProperty fooProperty() {
>>>        if (foo == null) {
>>>            foo = new SimpleStringProperty(this, "foo") {
>>>                @Override protected void invalidated() {
>>>                    if (!isInAppropriateState()) {
>>>                        throw new IllegalStateException();
>>>                    }
>>>                }
>>>            };
>>>        }
>>>        return foo;
>>>    }
>>> 
>>> The problem with this implementation is, throwing an exception from the invalidated() method results in both the exception being passed to the caller and the property value being updated with the new value. The latter is an undesirable side effect and, one can say, plain wrong behavior, even though it looks like some JavaFX classes find it acceptable, see e.g. Scene.camera property.
>>> 
>>> So I went on to searching JavaFX codebase and found that several JavaFX classes solve the problem by stashing the last valid property value somewhere and resetting the property to that value when throwing the exception, as follows:
>>> 
>>>    private StringProperty foo;
>>>    private String oldFoo;
>>> 
>>>    public final StringProperty fooProperty() {
>>>        if (foo == null) {
>>>            foo = new SimpleStringProperty(this, "foo") {
>>>                @Override protected void invalidated() {
>>>                    if (!isInAppropriateState()) {
>>>                        if (isBound()) {
>>>                            unbind();
>>>                        }
>>>                        set(oldFoo);
>>>                        throw new IllegalStateException();
>>>                    } else {
>>>                        oldFoo = getFoo();
>>>                    }
>>>                }
>>>            };
>>>        }
>>>        return myValue;
>>>    }
>>> 
>>> Examples of this approach include Animation.rate property and Effect.EffectInputProperty class.
>>> 
>>> I must admit I don't find this approach exceptionally elegant, but I may be missing something and there may indeed be a more elegant solution. So my question is, is this approach the recommended one?
>>> 
>>> Thanks,
>>> -- Vasiliy
>> 
>