[jira] Issue Comment Edited: (THRIFT-529) Change generated constructors so that application code evolves better

["Jonathan Ellis (JIRA)" <ji...@apache.org>]

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

Jonathan Ellis edited comment on THRIFT-529 at 12/4/09 5:30 PM:
----------------------------------------------------------------

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 shit.)

      was (Author: jbellis):
    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.
  
> 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.