Swagger-codegen: [JAVA] propagate fluent setters from jersey codegen to other java codegens

[ebelanger]

Description

A builder pattern when creating a complex request content would facilite usability.

Swagger-codegen version

2.1.6

Suggest a Fix

I would propose something like:

public Model withProperty(PropertyType property) {
    this.property = property;
    return this;
}

I will at one point probably create the mustache templates and would be able to provide them if this issue is accepted.

[wing328]

@ebelanger your suggestion sounds good to me as it's non breaking and developers who prefer the builder pattern would also benefit from this.

cc @cbornet

[cbornet]

IMO a builder is mostly needed when you have an immutable POJO (without setters). Since we generate the setters for all the properties, I don't think there is a big gain here...

[ebelanger]

I would disagree with you on that point. A builder pattern helps when creating a complex data structure that has multiple levels since it removes the need for variable declaration for nested objects.

new Person()
  .withAddress(new Address()
    .withLocation(new Location()
      .withLongitude(10.35)
      .withLatitude(11.74)));

It is equivalent to a cascade operator that is present in some languages like Dart for example. It's also really nice when it is the last parameter of a method since again it removes variable declaration.

api.postPerson(new Person()
  .withAddress(new Address()
    ...));

[cbornet]

You can also do that by returning this in the setter (fluent setter).
Also, my personnal taste for builder method naming is more

new Person.builder()
    .address(new Address.builder()
        .location(new Location.builder()
            .longitude(10.35)
            .latitude(11.74)
            .build()
        ).build()
    ).build();

My main concern with the builder pattern is that it adds a lot of quasi-duplicate code so if it can be avoided the better.
I think fluent setters work with Jackson and Gson, and if it does it would be indeed a great idea to use them. I'll give it a try.

[cbornet]

Actually the fluent setters are already generated together with the standard ones so you can already do

new Person()
    .address(new Address()
        .location(new Location()
            .longitude(10.35)
            .latitude(11.74)));

[cbornet]

Not that fluent setters is even more compact than builder since you don't have to call build() (that is missing from your code snippet btw :wink: )

[ebelanger]

I guess I meant fluent setters then since I was instantiating a new Person() and not a new Person.builder() :)

What configuration did you use to generate the fluent setters though? They don't appear when I generate with retrofit2.

Also is the JavaBean spec able to detect the pattern .address() and .address(Address) as a java bean property for instrospection concerns?

[cbornet]

Yes, it seems to be only in jersey model. I guess we need to propagate to the other java codegens.

[cbornet]

@ebelanger can you change the title of your issue ?

[ebelanger]

Will do. I edited my previous post and would like your opinion on the introspection aspect if possible :) It's pretty important for us since we use libraries for mapping generation that depends on the JavaBean specs.

[cbornet]

Both the JavaBean setter (returning void) and the fluent setter are generated so introspection should work just fine (it works at least with Jackson since the jersey codegen works well)

[wing328]

@ebelanger PR merged into master. Please pull the latest to give it a try.

Thanks for the PR from @cbornet

[ebelanger]

It is working great :) A big thank you to @cbornet :)

[smmcneill]

Is there a way to disable the generation of builder methods in model classes? These methods are causing problems for me.

[fehguy]

Please open a new issue and explain what is causing the problem

[jeep-guy]

I know this is an old thread, but I disagree with @cbornet that fluent setters are equivalent or superior to the builder pattern. The issue with fluent setters is that my Object is forced to be mutable, but with a Builder pattern, they both can and should be immutable. This guarantees that my Object is not being changed over time. Additionally, the build() method can be used in a Builder pattern to check for null required parameters, causing a quick failure without the need to add checks in each implementation method which requires those parameters later down the line. Instead, upon calling build() with null parameters, an exception would be thrown immediately. This is also beneficial, because when passing around a partially built Person Object into different methods, it’s not immediately clear that it’s only partially built and unready for use, but by passing a Person.Builder, that much is inherently clear to all developers.

As a user of Swagger codegen, I think it would be extremely useful to have the option to switch between these two options: fluent setters vs builders. Builder can be implemented extremely easily in Java by simply adding Lombok annotations to generated classes.

[jkoorts]

My 2cents but the builder pattern does not have any use for me when it is a response object. Responses comes in through the wire and is mapped by jackson. Jackson uses setters. In a unit test where you mock up a response object - could be the only place where I would really think a builder is a neat idea. Would be nice to switch off this feature