GEP: Adding Bound Properties to GroovyBeans

I see no reason why the setter should be illegal for bound properties. Why not simply wrap it if it exists?

Just add two flags to @BoundProperty:

@BoundProperty(checkMod=true, before=true)

checkMod will add this code:

if (this.x != newVal) {
...
}

before move the call of the setter before the check (so you can modify the value):

def oldVal = this.x
callOldSetter(...)
if (this.x != oldVal) {
... invoke listeners ...
}

It would be nice if one could set the default values for both at the class level to avoid repeating code.

A new 1.4 safe version of the bound properties. BoundProperty is pre-defined as a built in annotation, and class resolution is trapped out to use the internal definition, and error checking is told not to flag it as an error. Hence code that has the @BoundProperty annotation can be used in 1.4, both written and compiled.

Regarding check mod, the PropertyChangeSupport already traps out such non-changes and doesn't fire change events for non-modifying sets.

As for the before annotation, if we write the code for that I don't see why it should be optional. If we are wrapping a non-bound super method then the wrapped portion should only do the event notification. However, this conflicts with the GroovyBeans way of generating the field and the getter/setter. We would also need to overwrite code to not generate the field, and IMHO at that point you may as well just write an overriding setter that generates the events. Such a situation is rare enough the explicit coding of the setter would have some value in clarity of understanding.

My comment refers to the situation, when you need to do additional processing in the setter (for example, convert a String to an int):

class A {
    int x

    void setX (val) {
        if (val instanceof String) {
            x = Integer.parseInt(val)
        } else {
            x = val
        }
    }
}

In this case, you can't compare the value before the custom setter has run. In other cases, the setter can be expensive and you want it to stop early (i.e. compare first and not call the custom setter). So in my book, we need to be able to handle these three cases:

• Setter is generated by Groovy
• Setter is supplied by user; check for changes and notification added at the end
• Setter is supplied by user; check needs to wrap the whole setter

I'd also opt for checkMod=false so you can disable the compare for expensive objects (which would otherwise start to load stuff from a database, for example).

The bytecode generated foe setX(val) would actually generate a setX(Object) instead of a setX(int) which is what is needed for the JavaBeans sepecification. JavaBeans was based on an explicitly typed system. In reality the groovy would deletage the 'x =' lines to the setX(int) methods, so in this example there really wouldn't be any conflict.

you wrote:
"""
A compile time error MUST be raised when a class has any properties declared as a @BoundProperty and any or all of the five automatically added methods are also declared in the class.

A compile time error SHOULD be raised when the @BoundProperty is used on a field with declared visibility, hence not a GroovyBean Property.
"""

That's funny, because I would go another policy... if it is used on a field -> error, if getter/setter are used -> error, if any of

addPropertyChangeListener(PropertyChagegListener)
removePropertyChangeListener(PropertyChangeListener)
getPropertyChangeListeners()
addPropertyChangeListener(String, PropertyChangeLsitener)
removePropertyChangeListener(String, PropertyChagneListener)
$propertyChangeSupport

is used, I would not throw an error. I would even go as far as not using $propertyChangeSupport as field name, but propertyChangeSupport. Maybe even allowing to customize the field name through the annotation? Maybe it should be possible to apply the annotation to the class too andlet the user specify the field there.

One thing about the "patch"... first, thx for it. second, don't be angry if I have to modify that big times... the place for the code change is wrong and several other things that don't fit the usual way. Or do you want to change that? Then we would have to discuss how to.

I'm confused with "@BoundProperty is used on a field with declared visibility". Can you give an example here, please?

> setX(val) would actually generate a setX(Object) instead of a setX(int)

I guess this is true but still, why forbid users to write a fancy setter? Is there a good reason you can think of or just a general uneasyness about it?

Jochen:

The patch is a proof of concept. Tear it to shreds if necessary, I wanted to demonstrate how relativly low touch it was for Java 1.5. I was considering committing it myself, but after the large amount of discussion this seemingly small bit of code prompted I thought I should do a bit more of a formal process.

The 'should' on a field was mostly for ease of implementation. we can raise it to a must.

As to the exact semantics of the property change support, I was considering exposing a method 'protected void firePropertyChange(name, old, new)' in addition to the add and remove listener methods, and making the support private. That lines up with the way Swing handles it and would make extending swing properties easier. The two arg add and remove and getListeners is just nice to haves, not required, but JavaBeans requires the add and the remove to reflect properly. We probably should add some re-use logic when extending. Perhaps the rule could be fire(prop, old, new), add(listener), and remove(listener) must be present or it is an error, but if we are making our own add(prop, listener), remove (prop, listener), and getListeners() would also be generated.

Aaron:

It's not about forbidding users from write fancy setters, it's about playing nice with java.beans.Introspector. To allow that much free reigh we may need to generate BeanInfos for each annotated class. May be doable, but looks more like a 2.0 horizon item. Of course IIRC the Introspector relies on the order of the methods, so we could re-order the class file to make the generated get/set show up first or last (as needed)

The ordering of the methods is not fixed in the VM or for reflection. So doing logic based on the ordering the method has in bytecode is not a good idea.

This was addressed as part of the AST Transformations. Annotations are @Bindable and @Vetoable