Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@Builder should work with inheritance #78

Open
peichhorn opened this issue Apr 22, 2012 · 133 comments
Open

@Builder should work with inheritance #78

peichhorn opened this issue Apr 22, 2012 · 133 comments

Comments

@peichhorn
Copy link
Owner

Would be nice to also have a way to specify extra attributes in the builder to be used in the superclass constructor call.

Or if the superclass has a builder it would be nice use this builder when creating the builder of the subclass.

@peichhorn
Copy link
Owner Author

Issue #105:
Class A extends Class B with class A having a few fields and Class B addtional fields. How do I use the builder

Sadly this doesn't work for now. I still need to find a decent way to support this. If you already have ideas about this feature, please let me know.

@bennid
Copy link

bennid commented Oct 3, 2012

I definitely need this. I have a superclass in a dependency that I need for the subclass builder

@mrsquoosh
Copy link

Why not have the builder incorporate the fields from the super class if the super class is also annotated with @builder ?

@loicortola
Copy link

Hello @peichhorn,
first let me say I really enjoy working with Lombok, it's been making my life easier for months now.

I may have a suggestion for the implementation of the builder pattern, but I think there is still one obstacle to overcome.

The way you implemented it is to have a private inner class plus a public method, which doesn't support overriding. Instead, I'm suggesting we do a public static class, and the children's builders would extend their parent's

Here is how it looks like:

//File A.java

public class A {

    protected int myAttr;

    public static class Builder {

        protected A toBuild;    

        public Builder() {
            toBuild = new A();
        }       

        public Builder myAttr(int myAttr) {
            toBuild.myAttr = myAttr;
            return this;
        }       

        public A build() {
            return toBuild;
        }
    }
}

//File B.java

public class B extends A {

    protected int myOtherAttr;

    public static class Builder extends A.Builder {

        protected B toBuild;

        public Builder() {
            super();
            super.toBuild = new B();
            this.toBuild = (B) super.toBuild;
        }

        public Builder myOtherAttr(int myOtherAttr) {
            this.toBuild.myOtherAttr = myOtherAttr;
            return this;
        }

        @Override
        public B build() {
            return this.toBuild;
        }
    }
}

Let me know if this is of any help, and great job for everything

@takacsot
Copy link

I vote on this issue too. Somehow inheritance need to be handled.

@oantoshchenko
Copy link

+1 to the issue!

@arnaud-deprez
Copy link

+1 for this issue !

Any idea when a solution would be implemented ?

I think the solution described here is a good one and I've already used it in my code with success.

@fmucar
Copy link

fmucar commented Jan 7, 2015

+1 to the issue!

@Ruben-E
Copy link

Ruben-E commented Mar 10, 2015

+1

4 similar comments
@persapiens
Copy link

+1

@oloo
Copy link

oloo commented Mar 14, 2015

+1

@bartgeluykens
Copy link

+1

@ghost
Copy link

ghost commented Apr 8, 2015

+1

@oleg-korolenko
Copy link

+1 as well

@hexemeister
Copy link

+1

@ultrasecreth
Copy link

+1

There is a similar problem (from the user point of view) with @Accessor (#148)

@WonderCsabo
Copy link

👍

1 similar comment
@smougenot
Copy link

+1

@manicmonkey
Copy link

You're probably better off raising this with the core Lombok project as this repo has had no changes since March 2013.

@ultrasecreth
Copy link

Yeah, noticed that also, raised the issue in the official repo already :)

@arnaud-deprez
Copy link

I can't find the issue in the official repo anymore. It might help if you can provide the link :-)

@ultrasecreth
Copy link

Actually I started in the group (my bad on the ref) https://groups.google.com/forum/#!topic/project-lombok/-6b9dPH8qAw and I just saw that it will not be available in the short term :(

@fedotxxl
Copy link

fedotxxl commented Jun 7, 2015

+1

4 similar comments
@maxsap
Copy link

maxsap commented Jun 11, 2015

+1

@FranciscoE-Hudl
Copy link

+1

@wiecia
Copy link

wiecia commented Jul 27, 2015

+1

@dnno
Copy link

dnno commented Jul 27, 2015

+1

@Pola93
Copy link

Pola93 commented Jun 19, 2017

+1

6 similar comments
@SebastianDietrich
Copy link

+1

@wakawaka54
Copy link

+1

@lavcraft
Copy link

👍

@tfeiner
Copy link

tfeiner commented Aug 7, 2017

+1

@pengisgood
Copy link

+1

@ishiis
Copy link

ishiis commented Sep 8, 2017

+1

@jzaruba
Copy link

jzaruba commented Sep 8, 2017

...5 years later...
/cries/

@niks-
Copy link

niks- commented Sep 25, 2017

+1

4 similar comments
@mkalinowski2
Copy link

+1

@lichaojacobs
Copy link

+1

@lrodrigues-cit
Copy link

+1

@wyzssw
Copy link

wyzssw commented Nov 16, 2017

+1

@brnhrdt
Copy link

brnhrdt commented Nov 24, 2017

+1 comments are not speeding up this fix, but only spam inboxes of subscribed users. Please use reaction button instead. Thanks!

@Cesarla
Copy link

Cesarla commented Feb 13, 2018

👍

@maunzCache
Copy link

Uhm can we please discuss why lombok should support inheritance on a builder? Maybe i do not understand the builder pattern properly. What i'd expect is that every class must have it's own builder.

The inheritance you may want would rather suite a factory pattern. So you want to different things but made in a pretty similar way. Though inheritance may not by the best example for the pattern it would work.

Adding inheritance handling to a builder feels like abusing the builder pattern. I do not say that it is not possible or that it can have a positive effect for java oop but it is not behavior i'd expect when using a builder. Which means that when lombok would support such a mechanic a few users in this world are using patterns wrong and create misleading apis and that is where i see no benefit.

Please correct me if i think in the wrong direction but there might be a good reason why this issue is still open after five years.

@niks-
Copy link

niks- commented Mar 16, 2018

Will try to explain what I need.
It worth to mention at the very beginning that I am NOT good at OOP patterns.
I am looking at Builder as a very convenient way to construct objects - You don't need to use constructor with huge number of parameters, follow parameters order and provide value for parameters you want to keep default/null.

Let see example what I have.
I have class A (not extending other classes) with 10 fields (it is data object) and I am using Builder on it. It works grate.
Now I need class B which will extends class A with two additional fields (it is also data object). It would be very convenient to have Builder for this class too with ability to provide values for parent class fields and for own fields.
Creating class B with the same set of fields as class A + own fields looks like a code duplication and there is high probability you will forget to add new field to B when you will add it to A.

Maybe there is a pattern for above example (factory pattern doesn't look like solve this issue or I am missing something). I will be grateful if you show me it or suggest other way of the issue solving.

@maunzCache
Copy link

Why don't you add the class variables you need on B to class A? If you need to represent the object in a form that validates to A you'd then serve the default values which may be NULL or something else. In the case that you want your object to represent state B you use the corresponding methods on the builder.

It is also still valid to say that you want to use oop concepts like inheritance but first think about what makes B different from A. The builder pattern is only a creational pattern so it should only affect the way of instantiation. You may circumvent your problem with this solution. You will lose the builder on the parent class (A) but keep it on B. This is okay because it still fits my solution mentioned first. If you want to represent A you may use the builder on a representation of B which suits A. Consider:
A objectA = new B().foo(42).bar("Hello World").build();

The builder pattern does exactly what you want: Slim constructors because the class is very complex and very customizable. Its goal is not to reduce your boilerplate code.
(Right now i am sitting like 45 minutes on this comment and struggling with the english language to express what i have to say. So sorry if you don't find any arguments in this "discussion".)
What I am trying to say is that you're problem may not lay within the unsupported feature of lombok but in your class modeling. And from my point of view lombok should not support that feature because (for me) a builder is always connect to the idea of the builder pattern. So lombok protects you from misusing the pattern in not supporting that feature. It is not that it is impossible to create this kind of builder/feature but i'd rather forfeit it for cleaner code and general understanding of the pattern.

See Wikipedia for examples on that matter and for an explanation of the builder pattern.

Maybe we can find a use-case where it would be absolutely helpful to incorporate that feature but i cannot think of any right now. It should be more practical and concrete than class A and B.

This is only my opinion on the matter just consider it.

@janrieke
Copy link

janrieke commented Mar 17, 2018

To make something clear: We do not necessarily need inheritance on the builders. It's just the only solution we came up so far to solve another problem: We want the builder of our class to be able to set fields from superclasses.
When manually writing the builder, this would be a piece of cake (without the inheriting builders idea): Just add the respective methods to the (subclass') builder. Doing so is obiously no abuse of the builder pattern, because these fields are (by inheritance) members of the class we want to construct an instance of. We do not need any builder in the parent class to solve this problem.
You may also think of an abstract superclass, which will not allow having an own builder. Still we want its fields to be settable by subclasses' builders.

Why don't you add the class variables you need on B to class A?

This may be a solution in this example; however, there are problems where instances of A simply do not have these fields, for instance because there are other subclasses deriving from A.

The inheritance you may want would rather suite a factory pattern.

The factory pattern is used when you want to create objects without having to specify the exact class of that object (e.g., when you have an API that allows creating instances of an interface, but you want to make the concrete implementation an implementation detail and keep it interchangeable).
This is not the case here: We know what class we want, and we need to control which class instantiated.

@niks-
Copy link

niks- commented Mar 29, 2018

We want the builder of our class to be able to set fields from superclasses.

This is exactly what I meant. I need ability to set superclass' fields in subclass builder. Superclass can even not to have own Builder. It will not break Builder pattern (as far as I understand) because if I am creating Builder in vanilla java it is possible to set superclass' fields.

@SebastianDietrich
Copy link

The question is, why sould a builder not support class hierarchies? The builder pattern never prohibited classes in a hierarchy to be built.
The builder pattern focuses on constructing a complex object step by step, returning the object as a final step. Complex objects like e.g. domain objects are often found in class hierarchies. Builders should produce valid objects and as such they need to be able to set attributes of superclasses.

@maunzCache
Copy link

The valid argument for me is that the builder is not properly inherited. I was focused that the Pattern itself would be the troublesome point but what janrieke and niks- explained was that the inheritance of class variables is broken which must be supported. I really didn't wanted to flame this topic but find the detailed problem and a possible solution. I'll be quite from now on.

@janrieke
Copy link

We didn't mean to silence you. :)
These are valid questions, and you are not the only one questioning about the proposed builder-inheritance construction. I am currently discussing my PR with the Lombok maintainers, and a topic there is also that this approach makes the generated builder code difficult to comprehend.

@niks-
Copy link

niks- commented Apr 2, 2018

explained was that the inheritance of class variables is broken which must be supported.

@getJack I'm sorry if my words made you think so. I didn't mean that this is a bug and must be fixed immediately. I meant that it's nice to have such feature in Lombok Bulder if creators agree to it. I tried to explain why I want to it and how it will be convenient for me and just wrote down my opinion and my arguments. But decision is up to creators and maintainers. I'm not pushing. I will use Builder and entire Lombok anyway because it's cool.

@osamahaq
Copy link

+1

@janrieke
Copy link

As @SuperBuilder made it into Lombok a while ago, I think this issue can be closed.

@jl452
Copy link

jl452 commented Jun 2, 2021

i can't use @SuperBuilder in parent (it is library class)
i need repeat constructor from parent "as is" in my child

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests