[ 
https://issues.apache.org/jira/browse/THRIFT-529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12786028#action_12786028
 ] 

Jonathan Ellis commented on THRIFT-529:
---------------------------------------

Without naughty words:

[This breaks a ton of code. With all the vetos dreiss drops about how we can't 
make things behave sanely (see: utf strings, "default" arguments) because it 
would break C++ compatibility I'm blown away by how trivially you just broke 
almost every piece of java code out there.

> What if I have a non-optional field 1 and an optional field 64, then add a 
> non-optional field 2 (all the same type)? Old code that used the two-argument 
> constructor will have its semantics changed.

It's really a shame Java doesn't have a built-in way of marking methods 
deprecated so you can make api changes without screwing over existing users. 
Like maybe http://java.sun.com/javase/6/docs/api/java/lang/Deprecated.html. 
That would rock.

(To clarify, I'm suggesting that if you're convinced that only required 
constructures is The One True Way, which I am not, continuing to generate the 
all-fields constructors but marking it deprecated would at least warn people 
before breaking the world.) 

> Change generated constructors so that application code evolves better
> ---------------------------------------------------------------------
>
>                 Key: THRIFT-529
>                 URL: https://issues.apache.org/jira/browse/THRIFT-529
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>            Priority: Minor
>             Fix For: 0.2
>
>         Attachments: thrift-529-v2.patch, thrift-529.patch
>
>
> The constructors generated by the Java compiler encourage code that breaks 
> when the thrift definition changes. For example, it is common to add an 
> optional field to a pre-existing schema, like:
> struct Activity {
>   1: required i32 id;
>   2: required i32 type;
>   3: optional i64 timestamp; //newly added
> }
> Any code that used the Activity(int, int) constructor will now break.
> One option to address this problem is to only generate empty constructors. 
> However, this makes it cumbersome to create new objects as a line of code is 
> needed to instantiate each field. A second option is to generate constructors 
> only for required fields. For example, to create an Activity with a 
> timestamp, the user would need to do the following:
> Activity a = new Acitivity(3,4);
> a.set_timestamp(timestamp);
> This gracefully handles the addition of optional fields. For the case of 
> adding a new required field, the constructors would break. Arguably this is 
> desired behavior since all the code would need to be updated anyway, and this 
> way you would be getting compile errors instead of runtime validation errors.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to