Builder should support fields from parent classes

[lombokissues]

Migrated from Google Code (issue 818)

[lombokissues]

👤 elreydetodo   🕗 May 29, 2015 at 21:54 UTC

In a project I'm working on, I have an abstract base class with three sub-classes. Let's say that the base class has "id", "name", and "type" fields. I would expect one of the subclasses annotated with @ Builder to be able to set those fields in its builder implementation.

On a related note (though probably warranting a separate bug), @ AllArgsConstructor should probably take fields from parent classes into account.

I think it would be reasonable for this to be an option I have to pass to the annotation (i.e. @ Builder(includeFromParent=true)).

[lombokissues]

👤 Maaartinus   🕗 May 30, 2015 at 07:15 UTC

For Builder, see
https://groups.google.com/d/msg/project-lombok/1S7Y7sHXZS8/Rdzj_U4Tvi0J

For why it's not implemented yet, see
https://groups.google.com/d/msg/project-lombok/-6b9dPH8qAw/0rzZW6ZAgJIJ

Related problem
issue #740

[lombokissues]

End of migration

[smougenot]

Would be nice to include the workaround in the official documentation (on @builder)
ex :https://www.donneo.de/2015/09/16/lomboks-builder-annotation-and-inheritance/

@AllArgsConstructor
public class Parent {
  private String a;
}

public class Child extends Parent {
  private String b;

  @Builder
  public Child(String a, String b){
    super(a);
    this.b = b;
  }
}

public static void main(String[] ...){
  Child.builder().a("testA").b("testB").build();
}

[msarniak]

Builder pattern that supports inheritance without the chicken and egg problem - not the cleanest code, cluttered with generics, but since this is to be generated, maybe worth considering?

public class Parent {

    private final int a;

    protected Parent(final AbstractParentBuilder builder) {
        this.a = builder.a;
    }

    public static final class Builder extends AbstractParentBuilder<Parent, Builder> {
        public Parent build() {
            return new Parent(this);
        }
    }

    protected static abstract class AbstractParentBuilder<T extends Parent, B extends AbstractParentBuilder<T, ?>> {

        private int a;

        public B withA(int a) {
            this.a = a;
            return (B) this;
        }
    }
}


public class Child extends Parent {

    private final int b;

    protected Child(final AbstractChildBuilder builder) {
        super(builder);
        this.b = builder.b;
    }

    public static final class Builder extends AbstractChildBuilder<Child, Builder> {
        public Child build() {
            return new Child(this);
        }
    }

    protected static abstract class AbstractChildBuilder<T extends Child, B extends AbstractChildBuilder<T, ?>> extends Parent.AbstractParentBuilder<T, B> {

        private int b;

        public B withB(int b) {
            this.b = b;
            return (B) this;
        }
    }
}

With that, code below is valid:

Child child = new Child.Builder().withB(1).withA(2).build();

We may get rid of casts, adding some abstract self() method, implemented in concrete Builders.

[shakuzen]

I came here from peichhorn/lombok-pg#78, which has a ton of +1s. I hope the maintainers here are aware of this.

[fahran]

+1

[afedulov]

+1

[ywilkof]

+1

[electrobabe]

+1

[gaetancollaud]

+1

[troymolnar]

+1

[theoilie]

+1

[laxika]

+1

[jkocot]

+1

[Yeggstry]

+1

[rspilker]

includeFromParent=true is not trivial since it requires resolution. From now on, please use the thumbs up button under the first comment to share your vote.

[marcuss]

@rspilker there is a lot of other upvotes for this, but people made those in the wrong repo: peichhorn/lombok-pg#78 (comment)

[Insomnium]

+1

[hokkun-dayo]

+1

[christopher-baek]

+1

[iwaltgen]

+1

[rmordillo-witbooking]

+1

[xiemeilong]

+1

[jeffochen]

+1

[leanrobot]

+1

[driverpt]

+1.

Workaround

[gaetancollaud]

@driverpt Thanks for the workaround. But you have to generate the constructor for all fields, which can be huge for some entity classes! Too bad the @AllArgConstructor doesn't support inheritance either. 😝

[witek1902]

+1

[wingsofovnia]

+1

[janrieke]

This issue cannot be fixed without major changes to Lombok, i.e., deferring Lombok processing to after the sematic analysis of the compiler. Thus, we have investigated on workarounds to this problem. This is the solution we came up with.

Let's assume we have two classes: a superclass Parent, and a subclass Child.
It is impossible to access ANY kind of type information from the superclass while generating the Builder for the subclass. Therefore, we generate a builder for the superclass as well.
The idea is that we use this superclass builder to inject its values into the subclass instance.

Superclass:

@NoArgsConstructor
@Builder(builderMethodName="parentBuilder", applyOn=true) 
public abstract class Parent {
	@Getter @Setter
	private String parentString;

	@Getter @Setter @Singular
	private List<String> parentValues;
}

Subclass:

@Builder
public class Child extends Parent {
	@Getter @Setter
	private String childString;

	@Getter @Setter @Singular
	private List<String> childValues;
}

How to use:

Child child = Child.builder()
		.childString("childValue")
		.childValue("childValue1")
		.childValue("childValue2")
		.build();
Parent.parentBuilder()
		.parentString("parentValue")
		.parentValue("parentValue1")
		.parentValue("parentValue2")
		.applyOn(child);

Remarks:

• Immediate consequence of having builders for each class in the inheritance hierarchy is that only one builder can be used as the real builder that creates the instance using a constructor. All other builders just have to be "applied on" the existing instance.
• The applyOn() method relies on assigning values to the instance fields directly. If @Builder is placed on a constructor whose parameters are named differently than the class' fields, the applyOn() method cannot be generated.
• Builders must have different builderMethodNames in each class in the hierachy.
• We changed that @Builder also compiles on abstract classes, simply by omitting including the build() method.
• The name of the applyOn() method can be customized via annotation parameter.

We implemented this for the Eclipse builder, and it works quite well. :-)
@rspilker, @rzwitserloot: Is this something you would consider to include? If this is the case, we would continue working on that, especially by providing a javac implementation.

[el-dot]

You can invert Builder pattern and take builder instance as the param of constructor, this way Builder can be easily inherited, with added params in child class. So you could add @InheriteBuilder, some annotation field, or something like that to handle inheritace 'resolution' explicitly by user not compiler.

class A {
    final int x;
    A(Builder b) {
        this.x = b.x;
    }

    static class Builder {
        public int x;
        A build() {
            return new A(this);
        }
    }
}

class B extends A {
    final int y;
    B(Builder b) {
        super(b);
        this.y = b.y;
    }

    static class Builder extends A.Builder {
        public int y;

        @Override B build() {
            return new B(this);
        }
    }
}

I removed setters on builder for brevity;

[jonas-lauber]

+1

[disciple97]

+1

[zhxiaogg]

+1

[thekalinga]

Can you guys stop posting these useless +1 at the bottom, wasting both your and everyone else's time who have subscribed to this page.

If you want the issue to get more attention, post a thumbs up for the first post, not at the bottom

[janrieke]

I created a pull-request (#1337, hehe) that addresses this issue.

I followed the idea of @Krzychek mentioned before: Generate a constructor on the type that takes a builder as a parameter and uses the fields from the builder to set the fields of the new instance. The Builder.build() method calls this constructor.

The feature only works for type builders and must be explicitly switched on, in order to not break existing implementations, which may rely on the all-args constructor generated by @Builder.

Example:

@Builder(extendable = true)
public class Parent {
	@Getter
	private String parentString;
}

@Builder(inherit = true)
public class Child extends Parent {
	@Getter
	private String childString;
}

public class TestBuilder {
	public static void main(String[] args) {
		Child child = (Child) Child.builder()
				.childString("childStringValue")
				.parentString("parentStringValue")
				.build();
		assert("childStringValue".equals(child.getChildString()));
		assert("parentStringValue".equals(child.getParentString()));
	}
}

[janrieke]

It would be great to get some feedback from the project developers. If my PR has a chance of getting included, I'll fix the current conflicts in the PR to make it mergeable again, and/or improve the code if you have any remarks.
Furthermore, I could try implementing @msarniak's generics idea to get rid of the cast.

[thekalinga]

@rzwitserloot Any response to the above query?

[Komdosh]

+1

[kevcodez]

+1

[nedkoh]

Well the workaround(s) provided so far are not that useful nor great. Here is an example:
I'm trying to create inheritance with a contract from an interface and default implementation for properties to share and at the end have an immutable object. 3 things that help me accomplish that are: the inheritance (for providing the fields/constructor), the builder pattern (making things immutable), and lombok (elimination of writing boilerplate code).
What has been suggested is basically breaking at least one of my goals (and more likely 2). If I am going to write my own constructors I am not making things clean, saving much time with lombok, and I'm also defeating the purpose of inheritance.

[theoilie]

Agreed. There are still no ideal workarounds.

[nedkoh]

Please stop submitting +1 to the request - it only results in unnecessary comments and pollutes the post. If you agree/disagree please use proper comment/reaction/emoji above.

[zygimantus]

Still no progress in this issue?

[derfloh205]

+1

[faxxe]

+2 ;)

[copperfield-dev]

+1

[sumeettakalkar]

I see a lot of +1s, but is this issue fixed?

[nsidhaye]

@sumeettakalkar Issue is still open. #1337 having good discussion & solutions.

I am not seeing any clean solution. May be I need to use lombok without builder pattern as I my project is using heavy use of inheritance.

Previously I thought it would be easy or may be I can create some hack around @builder. Any way it is worth to watch #1337

@janrieke Thanks for PR.

[timothyBrake]

+1 please support this

[janrieke]

There is a new PR coming with a modified approach, as my first approach still required casting, which simply doesn't look good.
Posting those "+1" will not speed things up. Please just be patient for a little more.

[janrieke]

New pull request #1711 with an improved approach.

[vadipp]

Seems like #1711 is released as experimental in version 1.18.2, so this issue may be closed?