thrifty icon indicating copy to clipboard operation
thrifty copied to clipboard

Constructing Builders with nullable structs

Open jparise opened this issue 6 years ago • 3 comments

As I convert more and more (Java) code from Apache Thrift to Thrifty, I've started to run into a pattern where I start with an existing struct object that might be null and then "extends" that struct by setting some existing values. The most direct way to do this is by conditionally calling the correct Builder constructor:

public Struct updateStruct(Struct struct) {
    Struct.Builder builder = (struct == null)
        ? new Struct.Builder()
        : new Struct.Builder(struct);

    builder.foo(True);

    return builder.build();
}

It would be much more convenient if Struct.Builder accepted a nullable argument and added a if (struct != null) guard to the generated code so the above could just become:

public Struct updateStruct(Struct struct) {
    return new Struct.Builder(struct)
        .foo(True)
        .build();
}

That would detract a bit from the purity of the existing Builder interface, but this could be a case where caller convenience could take precedence.

jparise avatar Oct 02 '18 21:10 jparise

I'll also acknowledge that some larger refactoring exercises are in order, such as passing Builder objects through these calls instead of reified structs. Please take the above primarily in terms of an initial conversion and less so as a critique of the current API.

jparise avatar Oct 02 '18 22:10 jparise

The need for this certainly does feel like a sign that some refactoring is needed, but in any event I'm not opposed to convenience here.

benjamin-bader avatar Oct 04 '18 16:10 benjamin-bader

As an incremental improvement, #250 adds an explicit @NonNull annotation.

jparise avatar Oct 05 '18 14:10 jparise