You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Dave Engberg (JIRA)" <ji...@apache.org> on 2009/12/19 04:38:18 UTC
[jira] Created: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
JavaBean code generator produces incorrect setter methods
---------------------------------------------------------
Key: THRIFT-663
URL: https://issues.apache.org/jira/browse/THRIFT-663
Project: Thrift
Issue Type: Bug
Components: Compiler (Java)
Affects Versions: 0.2
Reporter: Dave Engberg
Priority: Minor
Attachments: java-setter.diff
The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
public void setFoo(int foo) {
this.foo = foo;
}
The more recent code in the 0.2.0 release now returns a value:
public MyStruct setFoo(int foo) {
this.foo = foo;
return this;
}
I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
public MyStruct setChainFoo(int foo) { ...
Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Updated: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by Bryan Duxbury <br...@rapleaf.com>.
I'd be willing to explore this option. This is just a more incremental
approach, I'd say.
On Tue, Mar 30, 2010 at 2:20 PM, Mathias Herberts <
mathias.herberts@gmail.com> wrote:
> On Tue, Mar 30, 2010 at 23:10, Bryan Duxbury (JIRA) <ji...@apache.org>
> wrote:
> >
> > [
> https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel]
> >
> > Bryan Duxbury updated THRIFT-663:
> > ---------------------------------
> >
> > Attachment: thrift-663.patch
> >
> > OK, how about this?
> >
> > The "beans" option will make sure the setter methods return void.
> >
> > The "private-members" option leaves the setter methods alone, but makes
> sure the actual members are private. This is for those of us who want the
> builder style setters but private instance variables.
>
> Why not come out all the way and have a 'builder' option which would
> create a Builder class which would do something similar as what J.
> Bloch describes in Effective Java (@see
>
> http://rwhansen.blogspot.com/2007/07/theres-builder-pattern-that-joshua.html
> )?
>
Re: [jira] Updated: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by Mathias Herberts <ma...@gmail.com>.
On Tue, Mar 30, 2010 at 23:10, Bryan Duxbury (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Bryan Duxbury updated THRIFT-663:
> ---------------------------------
>
> Attachment: thrift-663.patch
>
> OK, how about this?
>
> The "beans" option will make sure the setter methods return void.
>
> The "private-members" option leaves the setter methods alone, but makes sure the actual members are private. This is for those of us who want the builder style setters but private instance variables.
Why not come out all the way and have a 'builder' option which would
create a Builder class which would do something similar as what J.
Bloch describes in Effective Java (@see
http://rwhansen.blogspot.com/2007/07/theres-builder-pattern-that-joshua.html)?
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792810#action_12792810 ]
Bryan Duxbury commented on THRIFT-663:
--------------------------------------
For things like encoding thrift objects to XML, wouldn't it make more sense to use an TXmlProtocol or something? I don't think we should degrade the usability of Thrift objects to support an alternative serialization system.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851579#action_12851579 ]
Bryan Duxbury commented on THRIFT-663:
--------------------------------------
I'll try to tackle this today.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792808#action_12792808 ]
Bryan Duxbury commented on THRIFT-663:
--------------------------------------
Maybe it's because I'm not familiar with the JavaBean spec (or even their uses, apparently), but I introduced this as a syntactic sugar way to make instantiating objects easier. I would generally oppose unilaterally reverting these changes, and certainly would oppose generating a bunch of extra weirdly-named methods. I would however support adding a compiler switch that switches back to the old style if that's what people prefer.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Dave Engberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Engberg updated THRIFT-663:
--------------------------------
Attachment: java-setter.diff
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Ben Taitelbaum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792774#action_12792774 ]
Ben Taitelbaum commented on THRIFT-663:
---------------------------------------
This is also an issue when trying to serialize using XmlEncoder, since it violates the JavaBean spec.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury reassigned THRIFT-663:
------------------------------------
Assignee: Bryan Duxbury
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: java-setter.diff, thrift-663.patch
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Stephen Starkey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851576#action_12851576 ]
Stephen Starkey commented on THRIFT-663:
----------------------------------------
Any idea when this bug will be worked on? I've just started hitting a part of my application where this would be very handy (I.E., putting a REST front-end on some of my Thrift services). As it stands, I'll be applying the patch to my Thrift, but would love it if this thing got done.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury closed THRIFT-663.
--------------------------------
Resolution: Fixed
Fix Version/s: 0.3
I just committed this.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Assignee: Bryan Duxbury
> Priority: Minor
> Fix For: 0.3
>
> Attachments: java-setter.diff, thrift-663.patch
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Stephen Starkey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851647#action_12851647 ]
Stephen Starkey commented on THRIFT-663:
----------------------------------------
I was able to test the "beans" option -- works great! My YAML serializer loves this stuff.
Didn't test the "private-members" 'cause I don't have any code that needs it.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: java-setter.diff, thrift-663.patch
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Dave Engberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792877#action_12792877 ]
Dave Engberg commented on THRIFT-663:
-------------------------------------
Yes, sounds good.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792866#action_12792866 ]
Bryan Duxbury commented on THRIFT-663:
--------------------------------------
One of the changes that has been made is that we always generate the getters and setters now, regardless of whether you do java:beans or not. Maybe we should just make java:beans cause the methods to return void?
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Ben Taitelbaum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792862#action_12792862 ]
Ben Taitelbaum commented on THRIFT-663:
---------------------------------------
In general, I would agree with adding a new compiler switch so that we don't break backwards compatibility, but we're calling something a bean that's not technically a bean, which is why I would support breaking backwards compatibility in this case.
In terms of using a TXmlProtocol, this is the preferred solution if you're only using thrift objects, but in a system where the thrift object is used as just another java bean, I would expect that XmlEncoder should work.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-663:
---------------------------------
Attachment: thrift-663.patch
OK, how about this?
The "beans" option will make sure the setter methods return void.
The "private-members" option leaves the setter methods alone, but makes sure the actual members are private. This is for those of us who want the builder style setters but private instance variables.
Can you give it a whirl and see if it meets your needs?
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff, thrift-663.patch
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Ben Taitelbaum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792870#action_12792870 ]
Ben Taitelbaum commented on THRIFT-663:
---------------------------------------
I think that would work great for me.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Dave Engberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792863#action_12792863 ]
Dave Engberg commented on THRIFT-663:
-------------------------------------
The JavaBeans spec is available via:
http://java.sun.com/javase/technologies/desktop/javabeans/docs/spec.html
From 7.1:
For simple properties the accessor type signatures are:
void setFoo(PropertyType value); // simple setter
PropertyType getFoo(); // simple getter
...
I'd be ok with a compiler switch, but I think the default for the "-gen"
named "java:beans" mode should actually be a JavaBean (with a void
return value). If we want to have a forked generator that's not
actually JavaBeans, maybe it could be "-gen java:apache" or something
like that. The alternative would be a little odd ... a JavaBeans
generator mode that doesn't produce JavaBeans.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-663) JavaBean code generator produces
incorrect setter methods
Posted by "Stephen Starkey (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12851586#action_12851586 ]
Stephen Starkey commented on THRIFT-663:
----------------------------------------
Thanks! There's no huge rush -- it would take us some time to integrate the new version -- we have several apps using Thrift.
> JavaBean code generator produces incorrect setter methods
> ---------------------------------------------------------
>
> Key: THRIFT-663
> URL: https://issues.apache.org/jira/browse/THRIFT-663
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Dave Engberg
> Priority: Minor
> Attachments: java-setter.diff
>
>
> The original Thrift JavaBean generator produced set* methods for a property 'foo' that looked like:
> public void setFoo(int foo) {
> this.foo = foo;
> }
> The more recent code in the 0.2.0 release now returns a value:
> public MyStruct setFoo(int foo) {
> this.foo = foo;
> return this;
> }
> I can imagine this was possibly desired by someone to implement a "chaining" style of coding, but this is no longer a correct JavaBean. The JavaBean spec requires that the return type of set* functions be 'void', and various tools and frameworks enforce this requirement. For example, the Stripes web UI toolkit thinks that a field is read-only if the set* function doesn't return 'void'.
> I'll attach a trivial patch to restore this to the previous behavior. If a chaining-style setter is desired by others, I'd recommend making this a separate method on the bean rather than replacing the standard setter. E.g.:
> public MyStruct setChainFoo(int foo) { ...
> Or something like that.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.