peichhorn / lombok-pg

Collection of lombok extensions
http://peichhorn.github.com/lombok-pg/
Other
326 stars 44 forks source link

@BoundSetter should work with inheritance #109

Closed peichhorn closed 11 years ago

peichhorn commented 11 years ago

One solution would be to also create a delegate methods to the PCSs firePropertyChange(String, Object, Object) method. And to look for existing addPropertyChangeListener(PropertyChangeListener), removePropertyChangeListener(PropertyChangeListener) and firePropertyChange(String, Object, Object) methods in the type hierarchy before creating a new PCS. Same thing goes for the VetoableChangeSupport.

peichhorn commented 11 years ago

I'm basically done with this, but I will give it another look an cleanup a bit.

FlorianBruckner commented 11 years ago

There are still issues with inheritance and @BoundSetter

Assuming the following

public class Scene { @Getter @BoundSetter private String name; }

public class PlayableScene extends Scene { @Getter @BoundSetter private String duration; }

will generate a new field and support methods in both Scene and PlayableScene as private, where the field and the methods in PlayableScene will hide those in Scene. A listener registered in PlayableScene will never receive events for changes in Scene.

One option would be to look for a method getPropertyChangeSupport (which could not be private then) in the superclasses and use this instead.

Basically that is what I did for 0.10, where I created a superclass containing the @BoundPropertySupport infrastructure myself and let the subclasses use it.

peichhorn commented 11 years ago

Sorry I can't reproduced the issue. Here is what I did.

I put both Scene and PlayableScene in seperate source files. Then I used a decompiler to verify that @BoundSetter works as expected.

And this how they look like.

public class Scene {
  private volatile transient PropertyChangeSupport $propertyChangeSupport;
  private final Object[] $propertyChangeSupportLock = new Object[0];
  public static final String PROP_NAME = "name";
  private String name;

  public String getName() {
    return this.name;
  }

  private PropertyChangeSupport getPropertyChangeSupport() {
    if (this.$propertyChangeSupport == null) synchronized (this.$propertyChangeSupportLock) {
      if (this.$propertyChangeSupport == null) this.$propertyChangeSupport = new PropertyChangeSupport(this);
    }
    return this.$propertyChangeSupport;
  }

  public void addPropertyChangeListener(PropertyChangeListener listener) {
    getPropertyChangeSupport().addPropertyChangeListener(listener);
  }

  public void removePropertyChangeListener(PropertyChangeListener listener) {
    getPropertyChangeSupport().removePropertyChangeListener(listener);
  }

  public void firePropertyChange(String propertyName, Object oldValue, Object newValue) {
    getPropertyChangeSupport().firePropertyChange(propertyName, oldValue, newValue);
  }

  public void setName(String name) {
    String $old = this.name;
    this.name = name;
    firePropertyChange("name", $old, name);
  }
}
public class PlayableScene extends Scene {
  public static final String PROP_DURATION = "duration";
  private String duration;

  public String getDuration()  {
    return this.duration; 
  } 

  public void setDuration(String duration) {
    String $old = this.duration;
    this.duration = duration;
    firePropertyChange("duration", $old, duration);
  }
}

They look fine too me.

Than ran this little demo:

public class Main implements Application {

  @Override
  public void runApp(String[] args) throws Throwable {
    final PlayableScene scene = new PlayableScene();
    scene.addPropertyChangeListener(new PropertyChangeListener() {

      @Override
      public void propertyChange(PropertyChangeEvent evt) {
        System.out.println("'" + evt.getPropertyName() + "' old: " + evt.getOldValue() + " new: " + evt.getNewValue());
      }
    });
    scene.setDuration("foo");
    scene.setDuration("bar");
    scene.setName("a name");
    scene.setName("another name");
  }
}

Which prints out:

'duration' old: null new: foo
'duration' old: foo new: bar
'name' old: null new: a name
'name' old: a name new: another name

What am I missing?

peichhorn commented 11 years ago

Oh it's broken in javac.. sh*t.. thanks for the report.. I'm on it..

peichhorn commented 11 years ago

I'm going to track the progress in Issue #119.