Lombok: [FEATURE] VarArgs variants for @Builder

[kalexmills]

Describe the feature
Add an extra add method generated by @Singular, which accepts a varargs parameters -- thereby creating a nice(r) API which can also operate like List::of.

Lombokified:

@Singular
List<Type> things;

Delombokified:

class Builder {
/* all the methods currently added by Singular */

  public void things(Type... types) {
    this.things(List.of(types));
  }
}

Describe the target audience
Anyone currently using @Singular may find that the additional flexibility afforded by being able to replace builder().thing(a).thing(b).thing(c).thing(d). by builder().things(a,b,c,d) is quite nice.

Additional context
In #2000 a request was made for support for @VarArgs annotation on methods -- not involving the Builder methods generated by Lombok.

This idea was tangentially mentioned in #1279, but this feature request only targets @Singular.

In #1568 a question was raised as to whether method chaining current builder methods was better. The additional method calls suggested in that issue could be considered as extra boilerplate required by Lombok (which is boilerplate Lombok creates? 🤔)

[randakar]

You do realise that the entire point of the builder pattern is to make sure
people do not accidentally call .things() with the arguments in the wrong
order by having each field declared explicitly?
Because this is basically taking it a step backwards.

On Sat, Jan 12, 2019 at 5:52 PM K. Alex Mills [e-mail]
wrote:

Describe the feature
Add an extra add method generated by @Singular, which accepts a varargs
parameters -- thereby creating a nice(r) API which can also operate like
List::of.

Lombokified:

@Singular

List things;

Delombokified:

class Builder {

/* all the methods currently added by Singular */

public void things(Type... types) {

this.things(List.of(types));

}

}

Describe the target audience
Anyone currently using @Singular may find that the additional flexibility
afforded by being able to replace
builder().thing(a).thing(b).thing(c).thing(d). by
builder().things(a,b,c,d) is quite nice.

Additional context
This idea was tangentially mentioned in #1279
https://github.com/rzwitserloot/lombok/issues/1279, but this feature
request only targets @Singular.

In #1568 https://github.com/rzwitserloot/lombok/issues/1568 a question
was raised as to whether method chaining current builder methods was
better. The additional method calls suggested in that issue could be
considered as extra boilerplate required by Lombok (which is boilerplate
Lombok creates? 🤔)

—
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/2016, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCRRBQj69WMdE8CKCZwzAAl6cHaGR9ks5vChLMgaJpZM4Z8vaS
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

[kalexmills]

@randakar, I'm not sure that I follow. My proposal is only to remove the need for separate calls to Collection::add. It seems to me that there are three cases to consider, but please correct me if I am wrong.

Case 1) The collection annotated by @Singular is an ordered collection. In this case, we compare two usage patterns: builder().thing(a).thing(b).thing(c), which results in a series of invocations list.add(a); list.add(b); list.add(c);, as compared to builder.things(a,b,c) which would result in the invocation list.addAll(List.of(a,b,c)). The resulting state is the same in both cases.

I don't see how it's any easier or harder to get the order of arguments wrong in either one of these cases.

Case 2) The collection annotated by @Singular is an unordered collection like a Set. In this case, the order in which the operations occur won't affect the result.

Case 3) The collection annotated by @Singular is a Map. VarArgs do not help us in this case. This proposal was not intended to include maps.

If you'll pardon my straw-man, the only way I can understand your response is if the underlying implementations of the List or Set interfaces used produce side-effects along with calls to add. This can't be right, though, because it seems to me to be a strange use-case to want to support.

[randakar]

Ah. Never mind, I was mixing up a few things there.

On Sun, Jan 13, 2019 at 1:42 AM K. Alex Mills [e-mail]
wrote:

@randakar https://github.com/randakar, I'm not sure that I follow. My
proposal is only to remove the need for separate calls to Collection::add.
It seems to me that there are two cases to consider, but please correct me
if I am wrong.

Case 1) The collection annotated by @Singular
https://github.com/Singular is an ordered collection. In this case, we
compare two usage patterns: builder().thing(a).thing(b).thing(c), which
results in a series of invocations list.add(a); list.add(b); list.add(c);,
as compared to builder.things(a,b,c) which would result in the invocation
list.addAll(List.of(a,b,c)). The resulting state is the same in both
cases.

I don't see how it's any easier or harder to get the order of arguments
wrong in either one of these cases.

Case 2) The collection annotated by @Singular
https://github.com/Singular is an unordered collection like a Set. In
this case, the order in which the operations occur won't affect the result.

Case 3) The collection annotated by @Singular
https://github.com/Singular is a Map. VarArgs do not help us in this
case. This proposal does not include maps.

If you'll pardon my straw-man, the only way I can understand your response
is if the underlying implementations of the List or Set interfaces used
produce side-effects along with calls to add. This can't be right,
though, because it seems to me to be a strange use-case to want to support.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2016#issuecomment-453792488,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCRfMCGyhpy5iHz3688aWLWZL-lp_8ks5vCoDngaJpZM4Z8vaS
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

[rspilker]

Thanks for explaining the proposal with a Lomboked and a Delomboked example!

__TL;DR:__ We're rejecting this proposal.

There are a few problems with this. In random order:

• This is a backwards incompatible change. builder.things(null) currently resolves to builder::things(Collection<? extends Type> things). By introducing builder::things(Type... things), now the users have to add a cast to Collection<? extends Type>.
• We cannot use List.of in our generated code, due to compatibility with older java versions.
• The use-case is not so big. Instead of builder().thing(a).thing(b).thing(c).thing(d), you can already write builder().things(List.of(a,b,c,d))
• We don't think it is worth making this an opt-in, because that clutters our API and code

[mjustin]

This doesn't address the other three points (so it should still be rejected on the merits of those), but the List.of bullet point goes away if you replace it with Collections.unmodifiableList(Arrays.asList(things)).