peichhorn / lombok-pg

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

@BoundSetter - clarification and delombok #108

Closed fommil closed 11 years ago

fommil commented 11 years ago

I am using @BoundSetter within a JFrame (which already has bound property support).

However, I do not know if this is resulting in a new PropertyChangeSupport field being created or if it is simply using the existing firePropertyChange() method.

Furthermore, I tried to look at the code produced by delombok (I'm using maven-lombok-plugin 0.11.0.0 and lombok-pg 0.11.3-SNAPSHOT) but the @BoundSetter annotations are still there. Has the delombok aspect of @BoundSetter been implemented yet?

peichhorn commented 11 years ago

However, I do not know if this is resulting in a new PropertyChangeSupport field being created or if it is simply using the > existing firePropertyChange() method.

For now this would create a new PropertyChangeSupport field since @BoundSetter only looks for an a existing getPropertyChangeSupport() method in the source file. So nothing super smart is happening. My guess is, that this behaviour could lead to serious bugs, so I definitely need to look into this. In the meantime: Why do you have to subclass JFrame?

Furthermore, I tried to look at the code produced by delombok (I'm using maven-lombok-plugin 0.11.0.0 and lombok-pg 0.11.3-SNAPSHOT) but the @BoundSetter annotations are still there. Has the delombok aspect of @BoundSetter been implemented yet?

Delombok just prints the AST, so there is no need for a special delombok aspect. As for the maven plugin, I've never used it, but I'm going to try it out. In the meantime just use a decompiler to look at the generated code.

fommil commented 11 years ago

How about changing it so it takes a parameter to decide whether a PropertyChangeSupport is created or if the method is used instead? The J2SE pattern might actually be to expose convenience methods for the fire events, not to expose the PCS itself.

I'm extending JFrame because that's how the Netbeans GUI Editor likes to create the main application. (A moot point because the PCS is from Component, IIRC) I'm playing with a new design pattern that Lombok enables, which is to register visual components with themselves as change listeners, and then do all the MVC listener/handler registering in one place (if-else ing on the property name). e.g. setting the JFrame's controller will then pass the reference down to all sub components that need it, and register all the relevant views as listeners. MVC is great but I've always found the setup to be tricky and full of boilerplate.

Kind regards, Sam Halliday

Sent from my iPhone

On 20 Jul 2012, at 07:09, Philipp Eichhornreply@reply.github.com wrote:

However, I do not know if this is resulting in a new PropertyChangeSupport field being created or if it is simply using the > existing firePropertyChange() method.

For now this would create a new PropertyChangeSupport field since @BoundSetter only looks for an a existing getPropertyChangeSupport() method in the source file. So nothing super smart is happening. My guess is, that this behaviour could lead to serious bugs, so I definitely need to look into this. In the meantime: Why do you have to subclass JFrame?

Furthermore, I tried to look at the code produced by delombok (I'm using maven-lombok-plugin 0.11.0.0 and lombok-pg 0.11.3-SNAPSHOT) but the @BoundSetter annotations are still there. Has the delombok aspect of @BoundSetter been implemented yet?

Delombok just prints the AST, so there is no need for a special delombok aspect. As for the maven plugin, I've never used it, but I'm going to try it out. In the meantime just use a decompiler to look at the generated code.


Reply to this email directly or view it on GitHub: https://github.com/peichhorn/lombok-pg/issues/108#issuecomment-7123044

peichhorn commented 11 years ago

Thinking about it closely @BoundSetter does not work with inheritance at all right now, so I've created a new issue for this.

Issue #109 @BoundSetter should work with inheritance