Calling final builder step without providing required arguments

STEPS TO REPRODUCE Call final builder step (usually .build()) without providing required arguments

EXPECTED RESULT Compiletime error

ACTUAL RESULT Runtime error

MISC Instead of returning the builder on all steps, return the next required argument. When there are no required arguments left, return the builder. See eg http://blog.crisp.se/2013/10/09/perlundholm/another-builder-pattern-for-java

We thought about this for a long, long time.

We use as an example the Mail class, which has the following properties and requirements:

String from; // exactly 1
String subject; // exactly 1
List<String> to; // 1 or more
List<String> cc; // 0 or more
List<String> bcc; // 0 or more
String body; // at most once, defaults to empty string. (hence, 'optional')
int priority; // defaults to '2'.

Given the above, what your request is actually going to cause is the following, including the actual underlying aim as far as we can tell:

We get rid of the concept (in the type system) of a MailBuilder, replacing it with the concept of a MailBuilder with nothing set yet, a MailBuilder with only from set, all the way up to a MailBuilder which typesystem-wise guarantees that _from_, _subject_, and at least one _to_ has been set, at which point we can enable the build() method on this type. The goal: Have a type system that guarantees all mandatory fields have been set.
We railroad the build process, giving the caller ONLY the option to set the from, and once set, only the option to set the to, etcetera. The goal: Give the user of the API extremely simple, auto-complete based guidance on what to call, and what's mandatory.
Because of the railroading and use of the type system, a runtime and compiler (write time) error will occur if a mandatory property isn't set. The goal: If the builder is not properly applied, have a compiler error. Even if at the time it was written it was properly applied, but since then in a backwards-compatibility breaking API update, a new mandatory property has been added.

There are many downsides:

Because of the railroading you lose perspective: When I start building, I just know about from, I have no idea what other things I will be able to set on this builder. I'd have to put in a dummy from to see the next step; seeing it all is effectively impossible at least from the auto-complete dialog.
This concept does not work well with listy properties, and most things that are interesting to build have listy properties, as does this example. I can be forced to call to() at least once, but after that, the API should allow me to both (A) pile on more to() calls, for multiple recipients, or (B) move on to calling body.
Unless you want a combinatorial explosion of types, ALL the optional stuff (and there's really no such thing, there are default values though. If you want to split hairs, 'defaults to null' is sufficiently similar to 'optional') HAS to come at the end, which is annoying if there's a certain 'flow' to the process. In Mail, for example, it's just weird that you HAVE to apply the properties in the order of from → to → optional more to → subject → any amount of cc/bcc/body/priority. Most people do NOT think about emails in that order. It's more: from → mandatory to → more to → cc → bcc → subject → body.
Sometimes you want to toss a builder around to other methods, have each apply only that which they know about. I can easily imagine a situation where my code crafts a MailBuilder with subject, body, and from set, but which leaves to unset. I pass the builder to another method which sets the to and then mails it. This is not possible with this system!
You get lots of types that are cluttering up your namespace.

The downsides are inherent in this design.

Let's go back to the goals of this design and see if we can come up with something different that addresses these goals as well or better and has fewer downsides.

Type system safety: It's a lie. You get no such safety. Take the to here; it would be ridiculous if I cannot set the to() by way of an Iterable<String>, yet, if I can do that, what's stopping me from passing an empty list? Now there is no to set at all. Really, what the type system can do, and the only thing it can do, is guarantee that to() has been called at least once; it cannot guarantee that it has been called with a non-null value, or an empty iterable, or an invalid mail address. There's no safety to be had, all you can get is an acknowledgement that the coder who wrote the code at least didn't forget about its existence. That's probably worth something, but it's a lot less impressive a promise as 'it's all valid'.
Guidance: Yes, this does an adequate job of showing what to do, though, we need some IDE assistance here: We also get all the methods of java.lang.Object and we surely hope our auto-complete dialog gives those the appropriate downgrading in visibility and importance in that list. You're also railroaded; you don't get the complete list of mandatory properties, only the next one in the chain.
compiler errors if misapplied: Yup, this design delivers.

Clearly, type system safety can't be a goal because it is impossible. It is in fact an anti-goal: Using the type system for this suggests type safety, but that's not actually what you get, and misleading code is something that should be avoided for obvious reasons!

So, we're left with guidance and compiler errors.

Why not just... do that then? Stop abusing the type system for things it wasn't designed for. Why not have an IDE plugin that puts the mandatory properties you haven't set yet at the top of the auto-complete list and writes them in bold, and puts properties that can only be set once and are already set lower, and in grey? Have this same plugin flag your incomplete builder that you nevertheless call build() on, with a compiler error that specifically explains what you've done wrong. Grey out the build() call if one of the mandatories hasn't been set yet.

This requires only a few very simple annotations (@Mandatory, for example), and, of course, an IDE plugin. The plugin can automatically figure out that you're using the builder in any way that isn't 'straight through', so that you can no longer compile-time ensure that all mandatory properties are set, and simply won't complain about those uses. It'll do what it can (primarily, prevent you from calling subject() twice in the same chain), and that's it.

This has all the upsides and none of the downsides:

No type clutter.
No misleading promise that the type system is writing cheques it can't actually cash in regards to 'the mandatory stuff has been set properly'.
No railroading: Callers can set the properties in whatever order they like.
Guidance: Callers are told exactly what they can set, and what they must set.
You can hand off a half-set builder to other code without issue.
Listy things are supported almost trivially (and very 'nicely': The call to to() will NOT grey out after you already called it once, whereas the call to subject() will)
You do get compile-time errors if you forget something.
You do get compile-time errors if an API update adds a new mandatory property.

The conclusion is that this 'railroaded builder pattern' is a strictly inferior workaround compared to what the real solution is, which is an IDE that understands this stuff.

As such, we don't want to add this to lombok. Instead, we want to build this plugin.

Big thanks for the detailed reply. I won't attempt to change your mind, but I'll elaborate a bit nonetheless.

Poor visibility due to railroading Certainly, though it's arguably pretty negligible point, and one could make the inverted argument against simple builders. (ie "OK, I can provide all this stuff, but which are actually required?")

Order is off Certainly an annoyance, though again it's arguably pretty negligible.

Collections can be empty Certainly, though this is the case irrespective of the style of builder (and constructors as well), and until Java can enforce at compile time types like eg "non-empty list" or "list with three elements" devs will simply have to keep this in mind when coding. On a related note, passing null as a required argument is also allowed by the compiler. (though there are non-vanilla means of mitigating that, such as eg IDE plugins)

Can't pass half-built things around Certainly, for cases where doing so is desirable, the builder in question isn't appropriate.

Lots of types I used to think negatively about lots of types, but have begun to consider it to be less of a problem.

To be clear, what was proposed was not replacing simple builders with the builder in question, but to simply add it as an option to be used in cases where it has its uses. It's not a silver bullet to be used at all times. The basic idea is to combine the convenience of builders with the same degree of safety as constructors.

However I understand and appreciate your position, and I'd welcome other means of achieving the desired outcome, such as eg a plugin.

Unless you want a combinatorial explosion of types, ALL the optional stuff (and there's really no such thing, there are default values though. If you want to split hairs, 'defaults to null' is sufficiently similar to 'optional') HAS to come at the end, which is annoying if there's a certain 'flow' to the process.

@rzwitserloot That's not true in this case as you can easily do the following:

interface InitialBuilder {
    BuilderWithTo add(Recipient to);
}

interface BuilderWithTo {
    BuilderWithTo to(Recipient to);
    BuilderWithTo cc(Recipient cc);
    BuilderWithTo bcc(Recipient bcc);
    BuilderWithSubject subject(String subject);
}

and it costs no single additional type. The trick uses a single lombok-style private builder, which is the only non-interface involved and implements all these interfaces (simply declaring them does the job). With casting you can break out of the railroad, but without it you must follow the path.

But the idea is orthogonal to the trick: You only need to differentiate between "to is still missing" and "to has been given". Once in the second state, you can add a to method to any state and it costs nothing but the method.

@Maaartinus Adding all the optional stuff at each step removes the guidance aspect; it is now no longer clear which properties are optional and which are required. The careful observer will note that you CAN figure this out by noticing that the return type of the subject method is different, indicating that is the way forward, but that's my point: That's a crappy IDE experience. You should demand better of your tools.

@androidfred You can keep saying 'I think' and 'negligible' but my point is: We can do better. Generating a railroaded builder pattern is strictly inferior. We can argue the level of inferiority at length, and there might be a point to that (if it is truly 'negligible', making an entire plugin is overkill), but.. well, let's just say I don't think it's negligible.

You mention that lack of railroading implies that it's hard to know what's required. This is false: As I said, the plugin will highlight (for example by bolding the relevant entries in the autocomplete list) all the mandatory fields. As long as you see bold stuff (which, naturally, are also at the top), you can't call build yet. Which will be appropriately grayed out to indicate that's not gonna work out right now.

+1 for "combinatorial explosion of types", because

User is not forced to follow the pre-given order of setters strictly (as imposed by the interface solution prestened at https://stackoverflow.com/a/41162327/873282 --> https://blog.jayway.com/2012/02/07/builder-pattern-with-a-twist/). Thus -1 for just BuilderWithTo, but +1 for BuilderWithFromToSubject, BuilderWithFromTo, BuilderWithFromSubject, ... ("combinatorial explosion of types")
The CI/CD pipeline can check for errors. (In contrast to some intelligent IDE auto completion which needs to be implemented in any IDE)

Not sure what prompts commenting on a feature request that was closed years ago, but since I was notified I figured I'd mention:

I still don't understand the perceived problem with combinatorial explosion of types. The suggested implementation does not lead to it.

Google went ahead and implemented this feature and it works fine, see https://github.com/immutables/immutables/issues/450 and https://github.com/immutables/immutables/issues/438

Note that it's an option for those who want compile time safe builders, no one is suggesting they be forced unto anyone.

In any case, in the years since this feature request was filed, my opinions on Builders, Lombok and Java in general has changed a bit.

I've come to believe more and more that builders are unnecessary and dangerous - in absence of strong reasons not to, by default, you should just use all-args constructors (which do enforce providing all attributes), and if using all args constructors is cumbersome, rather than adding builders, it's a smell that you should redesign your classes.

These days I mostly use Kotlin data classes, which provide a lot of ease of use and safety out of the box. (but I still instantiate them using just vanilla all-args constructors)

If I saw it correctly, it was opened until you closed it a few minutes ago. The big fat red "Closed" IMHO came from the LINKED issue "Feature: Allow fields to be specified only via builder's constructor".

Thus, just for the record, you disagree with "Effective Java Item 2". --> https://www.informit.com/articles/article.aspx?p=1216151&seqNum=2

Thank you for pointing to https://github.com/immutables/immutables/issues/438#issuecomment-251331853.

With the example, I am forced to use following code:

ImmutablePerson.builder().name(null).title(null).build();

I cannot do the other way round:

ImmutablePerson.builder().title(null).name(null).build();

To enable that was meant by @rzwitserloot by "exponential explotion": One has to offer interfaces for all possible combination of builder calls.

No, I don't disagree with Effective Java. If you're subjected to a class like

public class NutritionFacts {
    private final int servingSize;  // (mL)            required
    private final int servings;     // (per container) required
    private final int calories;     //                 optional
    private final int fat;          // (g)             optional
    private final int sodium;       // (mg)            optional
    private final int carbohydrate; // (g)             optional
}

all safety and good class design has already been thrown out the window, so using unsafe builders is just convenient.

The example is begging the question though, because it's a poorly designed class to begin with, and the mindset of always adding builders to everything literally encourages exactly that poor safety and class design.

It's exactly what I'm referring to when I say if using all args constructors is cumbersome, rather than adding builders, it's a smell that you should redesign your classes.

Some people will design classes poorly no matter what you do. There is no helping that. If someone is going to create 10 fields all of the same type the builder isn't going to help but nor is the all-args constructor.

Especially with lombok in the mix the differences between the two are not large enough to really matter to people like that.

Personally I am a big fan of the readability of builder calls, on the grounds that in reviews it will be really clear what is being passed to what.

The only downside is that changes to the class fields won't be noticed across the board by the compiler, which I assume is the danger you refer to

builder calls suddenly not setting new mandatory fields.

.. but new mandatory fields are always going to be dangerous, especially in public interfaces or models. Backwards compatibility is a thing. A lot of it can be mitigated with sane defaults for your fields, but..

Nonetheless. Like everything in programming, it's a trade-off. Nothing should ever be applied mindlessly.

On Tue, Apr 21, 2020, 00:34 Fredrik Friis notifications@github.com wrote:

No, I don't disagree with Effective Java. If you're subjected to a class like

public class NutritionFacts { private final int servingSize; // (mL) required private final int servings; // (per container) required private final int calories; // optional private final int fat; // (g) optional private final int sodium; // (mg) optional private final int carbohydrate; // (g) optional }

all safety and good class design has already been thrown out the window, so using unsafe builders is just convenient. The example is begging the question though, because it's a poorly designed class to begin with, and the mindset of always adding builders to everything literally encourages exactly that poor safety and class design.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rzwitserloot/lombok/issues/1202#issuecomment-616846939, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIERPOFGR53LOTBJP3SUTRNTEY3ANCNFSM4CRB3YKA .

projectlombok / lombok

Calling final builder step without providing required arguments #1202