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

Todd Lipcon commented on THRIFT-529:
------------------------------------

Hi Jonathan,

1) we're working on a release to alleviate these problems. See THRIFT-622 - I 
hope to roll an rc tarball this weekend (have too much other stuff to do to 
spend much office time on this)

2) I'm with Bryan and David on this patch. Using constructors with lots of 
arguments is dicey. Your reference to Deprecation doesn't solve anything - the 
issue is about when you have a constructor MyClass(String foo, String bar) that 
silently changes to MyClass(String bar, String baz) because the optional 
parameters change. The signatures are the same, so we can't have them both and 
just deprecate one.

Here's a proposal for a solution:

For the struct:
{code}
struct MyStuff {
  1:string foo;
  2:string bar;
}
{code}

we generate one constructor with all the required fields, and then a *private* 
constructor for all fields, including optional. We can then generate factory 
methods like:

{code}
public static MyStuff createWithFooBar(String foo, String bar) {
  ...
}
{code}

Since the factory methods would explicitly list all the parameter names, we 
don't have to worry about silent breakage - any breakage would cause 
compilation errors. We could generate these factory methods for (a) all 
required fields, (b) all fields, and (c) anything else people would find 
useful. I don't think we should generate all combinations of fields - that 
would be silly.

Thoughts?

> 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: 529-revert.patch, 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