Feature proposal: builder with final fields guaranteed to be filled (at compile time)

[Lorenzo Puglioli]

@Builder is a great feature, but it has a big drawback: even if some field is final and required to be passed in the constructor, using this pattern is it not guaranteed that all of them are filled: is it possible to forget some; also if a new required field is added in a second time, no compilation error is raised.

So I propose a new pattern implementation that will result in a compile time error when a required field is missing.The use of this new pattern can be optional for backward compatibility.

The key innovation is the presence of multiple builders: one for each of the required fields and one for all the optional ones. For required fields the filling is constrained, for optionals is as usual.

Here a sample implementation.

import lombok.Getter;

import lombok.Setter;

public class RequiredFieldsDemo {

@Getter

private final String requiredField1;

@Getter

private final String requiredField2;

@Getter

@Setter

private String optionalField1;

@Getter

@Setter

private String optionalField2;

public RequiredFieldsDemo(String requiredField1, String requiredField2, String optionalField1, String optionalField2) {

this.requiredField1 = requiredField1;

this.requiredField2 = requiredField2;

this.optionalField1 = optionalField1;

this.optionalField2 = optionalField2;

}

public static RequiredFieldsDemoBuilder$requiredField1 builder() {

return new RequiredFieldsDemoBuilder$requiredField1();

}

public static class RequiredFieldsDemoBuilder$requiredField1 {

RequiredFieldsDemoBuilder$requiredField1() {

}

public RequiredFieldsDemoBuilder$requiredField2 requiredField1(String requiredField1) {

return new RequiredFieldsDemoBuilder$requiredField2(requiredField1);

}

}

public static class RequiredFieldsDemoBuilder$requiredField2 {

private final String requiredField1;

RequiredFieldsDemoBuilder$requiredField2(String requiredField1) {

this.requiredField1 = requiredField1;

}

public RequiredFieldsDemoBuilder$optionalFields requiredField2(String requiredField2) {

return new RequiredFieldsDemoBuilder$optionalFields(requiredField1, requiredField2);

}

}

public static class RequiredFieldsDemoBuilder$optionalFields {

private final String requiredField1;

private final String requiredField2;

private String optionalField1;

private String optionalField2;

RequiredFieldsDemoBuilder$optionalFields(String requiredField1, String requiredField2) {

this.requiredField1 = requiredField1;

this.requiredField2 = requiredField2;

}

public RequiredFieldsDemoBuilder$optionalFields optionalField1(String optionalField1) {

this.optionalField1 = optionalField1;

return this;

}

public RequiredFieldsDemoBuilder$optionalFields optionalField2(String optionalField2) {

this.optionalField2 = optionalField2;

return this;

}

public RequiredFieldsDemo build() {

return new RequiredFieldsDemo(requiredField1, requiredField2, optionalField1, optionalField2);

}

}

}

[Reinier Zwitserloot]

Yes, this is a somewhat common pattern and has been discussed at length. It adds a lot of complication, has various drawbacks (you have to follow the exact order, and hence makes it difficult to use the builder as its own object (i.e. passing it around to helpers and such). A better fix is with an IDE plugin. Which you're free to write - that feels like something that isn't quite what lombok should be doing.

[Lorenzo Puglioli]

In my opinion, the use of an IDE plugin is not a practical solution and, if the fields are required, it could be acceptable to force to fill them in the same place (and order) without passing the builder around (and also, the use of this pattern could be optional, with an annotation parameter).

But I catch your point... I'll read the old conversations on this argument.

Thanks for your answer.

[Emil Lundberg]

For what it's worth, you certainly can implement this pattern with @Builder as is, it just takes a bit of extra effort. For example, I do this in the java-webauthn-server library, in essentially the way you've outlined above: by returning intermediate Builder wrappers that expose one setter at a time and do not expose the .build() method until all required fields are set. Lombok's @Builder still helps out with filling the actual implementations - the wrappers just proxy the base Builder, and return the base Builder once the required fields are set.

I agree that this pattern is probably better implemented by the application rather than Lombok itself. In addition to the complications mentioned, this also has a fairly important benefit: it ensures that the order of operations can't suddenly change due to a change in Lombok's code generation (intentional or not), or a change in the order the fields are defined (which should be an implementation detail that doesn't affect the public API of the class), or something silly like that. It also forces you to carefully weigh the benefits (improved enforcement of correctness) against the drawbacks (more fragile API) and not just flip a switch without much thought.

Emil Lundberg

Senior Software Engineer | Yubico

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/f503fb58-a264-4ae7-b5de-e0a2cbb78391n%40googlegroups.com.

[Marc Günther]

Hi Lorenzo,

That sounds exactly like what Immutables implements, they call it "staged builders" or "telescopic builders":

- https://immutables.github.io/immutable.html#staged-builder

It comes with all the problems that Reinier pointed out, implementation dependent forced ordering of mandatory arguments, and it generates a new interface for each of these arguments. First problem was more or less a show stopper for us, so we don't use that specific feature, as much as I like my compiler to tell me about problems, and not defer them to runtime.

We use Immutables library since several years now. I chose it initially over Lombok, because it seemed less of a hack, but it turned out that isn't true. Like with Lombok, we had several of these "compiles fine with Eclipse, but completely fails in unexpected ways with javac" problems.

We later introduced Lombok as well, as I desperately wanted Extension Methods. Both libraries get along just fine.

Marc

[Joseph Ottinger]

What if mandatory fields were part of the Builder constructor itself?

Imagine a data class D with fields a, b, and c, such that a and b needed to be populated:

var dataInstance = D.Builder(aValue, bValue).build() // dataInstance' c is optional, so it gets null here
var otherData = D.Builder(aValue, bValue).withC(cValue).build() // now it has all three

I'm typing all this from my head, so the syntax might be incorrect for Lombok as it is, but hopefully the idea is communicated clearly enough. The builder would be unable to be constructed without values for a and b, and the compiler WOULD catch that; it might not be able to handle cases cleanly where cValue is supplied more than once, but I imagine the value of THAT would be pretty minor.

Thoughts?

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/79d2b411-f26e-4f95-a647-9c3bd2956841n%40googlegroups.com.

--

Joseph B. Ottinger
http://www.enigmastation.com

To the beautiful and the wise,

  The mirror always lies.

[Emil Lundberg]

Yes, this is a good compromise, at least if the parameters have different types that cannot reasonably be confused. If several or all of the parameters are Strings, for example, this approach loses the builder pattern's benefit of emulating named parameters to prevent argument order confusion.

Emil Lundberg

Senior Software Engineer | Yubico

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAEhvUyzA0eRK5wL5sVvJ9Vi4EqNPWTa_6teRORWBd-r1cgGbGA%40mail.gmail.com.

[Joseph Ottinger]

You could have a builder builder! :D

var dataInstance = D.BuilderBuilder.withA(aValue).withB(bValue).build().withC(cValue).build() // woohoo!

For the record, this is HUMOR (or what passes for it in my office) and isn't to be taken seriously. Your concerns are valid, if the types match you do lose the potential context of which value goes where, but I think it's a potential solution that solves MOST of the problem if not all of it. (On the other hand, I've not had the pain of the constructed scenario often enough that I've really wanted a solution, although I can see some value in it.)

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CANMnvkzkpJEgnznvLq-jFEp2psdH03%2BxzCdu5FyLsLtrkmU4wQ%40mail.gmail.com.

[Lorenzo Puglioli]

The builder for mandatory params can be useful in cases of a small number of required paramst against a large number of optional params.

Otherwise, as you already pointed out, the builder has no advantages against a traditional constructor.

Regarding some type of 'staged builder', I can only speak from my own experience, according to which the problems you mentioned are more theoretical than practical.

In my code I have often the need of filling pojos to be used as arguments/configuration params for other classes/modules, and usually they contains many mandatory fields.

- in the 90% of use cases a lombok "staged builder" would be perfect (no mixed params, no missed params... it could be a slogan :) ) and fit the need, because the potential limits are not a concern

- in the remaining 10% of use cases I could switch to the traditional builder

Do you think these percentages are realistics, or my codebase is statistically a too small sample?

And, even if the percentages are different (maybe even 50% and 50% ?) don't you think it would be worth enough to consider it?

@Mark: the Immutable library seems very interesting, but I'm not sure I want add another Annotation preprocessor on my project: I already use lombok and aspectj, and I already have had some difficult to make them work together!

[Michael Ernst]

The Called Methods Checker of the Checker Framework can guarantee that all required arguments have been passed before the `build` method has been called.  It has several positive aspects:

• it does not require any changes to your code
• you may provide the values to the builder in any order
• a method can partially fill in a builder and then pass it to another method to provide more values.
• lombok support is built in

This does require you to run a second annotation processor (the Checker Framework) in addition to Lombok.  And, if your use of builders in complex (such as passing partially-completed builders between methods), you will have to write a few annotations to specify the intended behavior.

If you use the Gradle build system and the io.freefair.lombok plugin, running the Called Methods Checker works out of the box.  Otherwise, your build system needs to run delombok, then run the Called Methods Checker on the delomboked code.

Disclosure:  I am one of the maintainers.  We know of many users who are using the Called Methods Checker successfully.  Give it a try and send us your feedback.

 

-Mike

[Rawi]

Is there a specific reaseon why you have to run delombok to use the checker framework? Is there anything lombok should change?

[Jan Rieke]

Is there a specific reaseon why you have to run delombok to use the checker framework? Is there anything lombok should change?

There has been some discussion on this issue a while ago here: https://github.com/typetools/checker-framework/issues/3387 

If my assessment is correct, there is a good chance that the problem will go away if we get rid of lombok's additional compilation rounds (https://github.com/projectlombok/lombok/issues/3509).

Jan