Generator-jhipster: [Proposal] Replace fluent API by the Builder pattern

[Tcharl]

Overview of the issue

Fluent API is not that good: it does not really fit all the need, particularly when using inheritance.
For example, when using this code:

public class Food {
private int calory;
public Food calory(int cal) {
this.calory = cal;
return this;}
}
public class IceCream extends Food{
private String taste;
public IceCream taste(String tst) {
this.taste= tst;
return this;}
}

We then can't do
IceCream cream = new IceCream (); cream.calory(100).taste("Chocolate");

Using the Builder pattern:

public class Food {
private int calory;


protected Food(Builder builder) {
this.calory = builder.calory;
  }
public static class Builder<T extends Builder> {
        private int calories = 0;
        public Builder() {}
        public T calories(int val) {
            calories = val;
            return (T) this;
        }
        public Food build() { return new Food(this); }
    }
}
public class IceCream extends Food{
private String taste;
public static class Builder<T extends Builder> extends Food.Builder<Builder<T>> {
        private String taste;
        public Builder() {}
        public Builder taste(String taste) {
            taste = val;
            return this;
        }
        public IceCream build() { return new IceCream (this); }
    }
protected IceCream (Builder builder) {
super(builder);
this.taste = builder.taste;
  }
}

Or Something similar (sorry, I haven't got any code editor at the moment) would do the trick and seems to be a little bit 'Cleaner' (separation of concern, Etc.).

Motivation for or Use Case

Handling inheritance, doing things right.

Reproduce the error

Trying inheritance with fluent API

Related issues

No.

Suggest a Fix

I can make a PR ;-)

JHipster Version(s)

4.5.1

JHipster configuration

Any configuration using fluent setters.

Let's discuss about it!

• [x ] Checking this box is mandatory (this is just to show you read everything)

[jdubois]

Would this work well with Hibernate?

[Tcharl]

As long as we keep an empty constructor I think so.
I've already successfully used this pattern in an OpenJPA project, I'll try to test within Hibernate ASAP.

[cbornet]

Well, it adds a lot of code... And since there's no inheritance between our domain classes, is there a big gain ?
Now it would be interesting to see if we could have immutable objects instead of javabeans. The builder would then be most needed and we would need to check if it can work efficiently with hibernate, jackson and mapstruct

[delverdev]

I would like to suggest "Java 8 style builder":

public class User {

    private String name;

    public User() {
    }

    public User(String name) {
        this.name = name;
    }

    public User(Consumer<User> builder) { //Here it is
        builder.accept(this);
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public static void main(String[] args) {
        User user = new User(u -> u.name = "John"); //Here is the usage
        System.out.println(user.getName());
    }

}

It's easy to create and it doesn't add a lot of code.

[Tcharl]

Looking at the last answer of this stackoverflow, I think we can assume that it is possible to make 'semi-immutable' beans. WDYT?

[pascalgrimaud]

Well, it adds a lot of code... And since there's no inheritance between our domain classes, is there a big gain ?

Agree with @cbornet, I don't see much value here. For me, it is a specific need.
Currently, I'm very happy with getters, setters and fluent setters.

What do you think about doing this as a module?
Your module could scan all domain classes and you can add / remove / change / replace what you want.

[atomfrede]

Using https://immutables.github.io looks like better idea than handcrafting builder code

[Tcharl]

I wasn't aware of this framework, looks nice :). What about trying to implement DTO with this?

[cbornet]

@atomfrede immutables.github.io looks like a great lib ! It has a "Modifiable" variant that could probably be used with Hibernate (need to check if it works with JSR303 validation) and would allow to remove our getter/setter/fluent generated code. And we could probably also use immutables for the DTOs
But this imposes to use an annotation processor which is not always very user-friendly (see Mapstruct...) so I'm not sure about this...

[atomfrede]

@cbornet do you think we can do it as a module? I don't think we can and should try to maintain it in the main generator.

[deepu105]

No more annotation processor please. Its annoying actually. And this one is
similar to Lombok so the same argument applies.

On Sun, 28 May 2017, 10:33 pm Frederik Hahne, [e-mail]
wrote:

@cbornet https://github.com/cbornet do you think we can do it as a
module? I don't think we can and should try to maintain it in the main
generator.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jhipster/generator-jhipster/issues/5830#issuecomment-304538432,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDlFxJUSC_PTZDMbqAINjrRpA03f8Qhks5r-doZgaJpZM4Nk4to
.

[elucash]

@deepu105 while there are some bugs in compilers, IDEs and build tools which make standard annotation processors not always a pleasant ride, but, please, do not compare compiler plugin (Lombok) which makes AST transformations (and violating JLS along the way) with standard annotation processors.

[deepu105]

Agree. Im not comparing them in that sense. I was talking from the
perspective of adding more annotation processors as we had a similar
discussion for lombok and decided against it so the same logic applies here

On Mon, 29 May 2017, 12:41 am Eugene Lukash, [e-mail]
wrote:

@deepu105 https://github.com/deepu105 while there are some bugs in
compilers, IDEs and build tools which make standard annotation processors
not always a pleasant ride, but, please, do not compare compiler plugin
(Lombok) which makes AST transformations (and violating JLS along the way)
with standard annotation processors.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jhipster/generator-jhipster/issues/5830#issuecomment-304545004,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDlF9BXCGUQ0usSuPJ_wRSo6ERh1eunks5r-fgXgaJpZM4Nk4to
.

[jdubois]

I'd love to have more of those technologies in JHipster, however they add a burden in setup and development workflow. They just don't work seamlessly out-of-the-box: for instance, we haven't been able to find a good way in the last years with MapStruct.
So, while those are definitely cool, they don't work well with our "developer experience" goal. So they can only be modules, or non-default options.

Anyway, back to the builder pattern: as @cbornet says, it adds a lot of code, for a pattern (inheritance) that we don't use by default (and which I wouldn't recommend with Hibernate, but that's another story). So for me we shouldn't put this by default.

Besides, the current "fluent" pattern has been validated by the Hibernate team, so I wouldn't want to touch that without a very good reason.

[atomfrede]

Totally agreee, it should not be default and from my point of view also not part of the main generator.

[jdubois]

OK, then I'm closing this -> @Tcharl is that fine with you?

[cbornet]

Maybe that would be useful for the DTOs ?

• DTOs currently don't have fluent methods
• DTOs/VMs would benefit from being immutable and I think Jackson can be easily configured to use the builder
• we already use an annotation processor so it's just about making sure they play nicely together

[gzsombor]

DTOs are either created by Jackson, or the generated MapStruct code, so being immutable, or having fluent methods are not that big improvement - you need to convince MapStruct and Jackson to follow the new style.

[cbornet]

@gzsombor quite true, indeed !

[Tcharl]

@jdubois Ok for me, I totally agree with the 'amount of boilerplate code' argument (but I don't really understand the 'near fluent setter' one).
@gzsombor @cbornet : I'm sorry, but what do Mapstruct or Jackson have to do with the Jhipster DTO template?
DTO are IMHO Immutables by definition so why can't we change the JH accordingly?

[cbornet]

The point from @gzsombor if I understood it well is that DTOs are created and filled either by Jackson or by Mapstruct and never directly by the user so immutability doesn't have a huge value there.
(Edit : I realize, I'm paraphrasing what @gzsombor already wrote...)

[Tcharl]

Dumb me!
We can tweak the Jackson way for generating Code from JSon, but the mapstruct counterpart looks more complicated.
I'll give the Immutable DTO subject a bump when these things will be easier.

Best regards and thank you all.

[cbornet]

@Tcharl I've done builder mapping in the jhipster grpc module ™️ (protobuf is all builders). See https://github.com/cbornet/generator-jhipster-grpc/blob/master/generators/entity/templates/_entityProtoMapper.java#L104 . I've not seen the issues they mention in your link...