JavaBeanPropertyAdapter

[Michael Heinrichs]

Hi Richard,

so many questions. :-) Answers are inlined.

- Michael


On 17.12.2011, at 00:34, Richard Bair wrote:

> Hi Michael,
> 
> On Dec 14, 2011, at 3:24 AM, Michael Heinrichs wrote:
> 
>> Hi,
>> 
>> I would like to add two new classes (javafx.beans.property.JavaBeanPropertyAdapter and javafx.beans.property.JavaBeanReadOnlyPropertyAdapter) to provide ready to use support for Java Bean integration (http://javafx-jira.kenai.com/browse/RT-14004). The names are just working titles, I am open to any suggestion.
> 
> The names are long but descriptive. I wonder if you just "ReadOnlyJavaBeanProperty" and "JavaBeanProperty" is sufficient? I like "ReadOnly" being the first of the name just from an english readability perspective.

Not sure, these classes are not really properties, but builders. Maybe JavaBeanPropertyBuilder and ReadOnlyJavaBeanPropertyBuilder are better names.

> 
>> Both classes allow to create adapters for conventional Java Bean properties and map them to JavaFX Properties.
>> 
>> The read only case is pretty straightforward. The Java Bean property has to be a bound, read-only property, i.e. it has to have a getter and support PropertyChangeListeners.
> 
> I like what the others were saying, where we also support beans without explicit property change listeners. Even if a JavaBean implemented the addPropertyChangeListener method(s), they might not actually be wired up for the property you are trying to listen to, so the fact that there is, or is not, an addPropertyChangeListener method doesn't really indicate whether the specific property you are trying to adapt will be observable.
> 
> A few more questions:
> 	- Do you check for both addPropertyChangeListener(listener) and addPropertyChangeListener(name, listener) methods?

Not right now, but so far I have only prototyped this. The final solution will check for the methods above plus add<PropertyName>Listener(listener).

> 	- Do you require a removePropertyChangeListener(listener) / removePropertyChangeListener(name, listener) equivalent? Or will you add listeners with no way to remove them later? Speaking of which, how *do* you remove listeners later? Are they all weak listeners?

Yes, so far I do require removePropertyChangeListener. I guess I can safely weaken this constraint. Though this may cause memory leak, because if there is no removeListener the JavaBeanProperty will be kept in memory until the JavaBean is GCed.

Listeners are removed either by calling unbind() or if the JavaBeanProperty goes out of scope. Internally a weak reference points from the Java Bean to the JavaFX Property. Unfortunately this can only be cleared if a change event is fired, because I do not have access to the Java Bean's internal workings.

> 	- Is there a public method on the Adapter classes that can be used to manually poke it and force it to fire a change event?

Nope, not yet. But if we weaken the constraints on the support for change listeners, we need to allow manually firing events. Have to take a look at this.

> 
>> I propose the following API:
>> 
>> public class JavaBeanReadOnlyPropertyAdapter<T> {
>>   public JavaBeanReadOnlyPropertyAdapter(Class<?> clazz, String propertyName);
>>   public JavaBeanReadOnlyPropertyAdapter(Class<?> clazz, String propertyName, String getterName);
>>   public JavaBeanReadOnlyPropertyAdapter(Class<?> clazz, String propertyName, Method getter);
>> 
>>   public ReadOnlyObjectProperty<T> createProperty(Object bean);
>> 
>>   public static <T> ReadOnlyObjectProperty<T> createProperty(Object bean, String propertyName);
>> }
>> 
>> To use it, one first has to create a JavaBeanReadOnlyPropertyAdapter, passing the class and the propertyName, if the Java Bean follows the Java Bean convention. If not, one can also pass in the name of the getter method or the getter directly. In a second step, one creates the property by calling createProperty(). The static method is for convenience, if only a single property is needed.
> 
> Why did you go this route? Actually I just noticed this, I had assumed the adapter was actually just a Property subclass. Is it so that you can produce a primitive version of each type? It seems awkward, I think I like JavaBeanBooleanProperty and so forth if you decide to have implementations that are type specific (just like we have SimpleBooleanProperty and so forth).
> 

The main reason is, that I want to limit reflection. I expect very often you want to listen for the same property in a collection of beans. With the current approach we have to look up the required methods only once. If each JavaBeanProperty is created individually, we have to look up the methods each time.

We can add type specific implementations, if you think we should. The advantage will not be as significant as usual, because we cannot avoid boxing, but at least it would give you the fluent API at your fingertips.

> What happens if I try to create a JavaBeanReadOnlyPropertyAdapter for a property which has no public getter?
> 

This would fail, throwing an exception.

>> The writable case is a little more complex, because we are not in full control of a Java Bean property. The Java Bean property has to be bound and writable. If you want to define a unidirectional binding for a Java Bean property, it also has to be constrained (i.e. support VetoableChangeListeners).
>> 
>> Bindings for Java Bean properties will be slightly different than our usual bindings: they will be eager and they may become out-of-sync. Let's say we have a Java Bean Property p, that is bound to an ObservableValue v. Internally we hook a listener to v and propagate all changes to p by calling its setter (that is why it will be eager). If now p does not accept the value from v, p and v will be out-of-sync. Especially the last point bothers my, but I guess there is no way to ensure synchronization.
> 
> Right, we're just sort of hosed there.
> 
>> The API and usage is similar to the read only case above. Only the method isBindable() is new, which returns true, if the Java Bean property is constrained and can therefore be bound unidirectionally. (If you try to define a unidirectional binding for a Java Bean property, that is not constrained, an UnsupportedOperationException is thrown in bind().)
> 
> Very few Java classes use VetoableChangeListeners. However, I think that is OK since the common usage here will be to either do a unidirectional binding from a JavaFX property to a POJO (for example, Label.text is bound to Customer.name), or a bidirectional binding between the two. So this seems fine.
> 
>> public class JavaBeanPropertyAdapter<T> {
>>   public JavaBeanPropertyAdapter(Class<?> clazz, String propertyName);
>>   public JavaBeanPropertyAdapter(Class<?> clazz, String propertyName, String getterName, String setterName);
>>  public JavaBeanPropertyAdapter(Class<?> clazz, String propertyName, Method getter, Method setter);
>> 
>>   public ObjectProperty<T> createProperty(Object bean);
>>   public boolean isBindable();
>> 
>>   public static <T> ObjectProperty<T> createProperty(Object bean, String propertyName);
>> }
> 
> Again, I don't know what the value is of the adapter, it seems like it would be more natural to just have JavaBeanBooleanProperty?

Same reason as above, if you want to listen for the same property on a huge collection of beans, you can run into performance issues. Maybe we need both worlds, a class JavaBeanBooleanProperty, that extends BooleanProperty and a JavaBeanPropertyBuilder that avoids reflection.

> 
> Thanks
> Richard