[FEATURE] @WithBy support (with, but with lambdas)

[rzwitserloot]

We already have @With. However, if you have hierarchical immutable data structures, it is unwieldy. Example:

@Value class Movie { @With Director director; }
@Value class Director { @With LocalDate dob; }

to modify the name of the director given a movie variable, you'd have to do: movie.withDirector(movie.getDirector().withDob(movie.getDirector().getDob().withYear(1999))); – add more levels of nesting and it gets a lot longer.

Proposal

lombokized:

@Value class Movie { @WithBy Director director; }
@Value class Director { @WithBy LocalDate db; }

generating:

public Movie withDirectorBy(UnaryOperator<Director> mapper) {
    return new Movie(mapper.apply(this.getDirector()));
}

Why?

Well, compare:

movie.withDirector(movie.getDirector().withDob(movie.getDirector().getDob().withYear(1999)));

movie.withDirectorBy(d -> d.withDobBy(b -> b.withYear(1999)));

as this is a bit of an experiment (using lombok to establish novel new idioms), let's start this one in the experimental package.

NB: Intentional choice to go with the withXBy naming scheme; we could just do withX just like @With does, and the 2 generated methods don't really clash in the sense that I expect having a field of type UnaryOperator is exceedingly rare, however, by having two equally named methods that both take a single reference, withX(null) would then be a compile time error (ambiguous invocation). To avoid that, we use a different name.

[randakar]

Good stuff. I like it. I presume this won't address reference loops. So given class A with field b, which contains a field named 'a' pointing the other way: a.withBBy( b -> b.withABy(... Whoops ...)); Kinda wondering if this would make that easier since that has been a historic issue when using builders to construct hierarchies like that, usually because of one-to-many relations in hibernate model classes.

…

On Thu, Feb 13, 2020, 23:10 Reinier Zwitserloot ***@***.***> wrote: We already have @with. However, if you have hierarchical immutable data structures, it is unwieldy. Example: @value class Movie { @with Director director; } @value class Director { @with LocalDate dob; } to modify the name of the director given a movie variable, you'd have to do: movie.withDirector(movie.getDirector().withDob(movie.getDirector().getDob().withYear(1999))); – add more levels of nesting and it gets a lot longer. Proposal lombokized: @value class Movie { @Withby Director director; } @value class Director { @Withby LocalDate db; } generating: public Movie withDirectorBy(UnaryOperator<Director> mapper) { return new Movie(mapper.apply(this.getDirector())); } Why? Well, compare: movie.withDirector(movie.getDirector().withDob(movie.getDirector().getDob().withYear(1999))); movie.withDirectorBy(d -> d.withDobBy(b -> b.withYear(1999))); as this is a bit of an experiment (using lombok to establish novel new idioms), let's start this one in the experimental package. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2368>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABIERJNEHHWLQSZX4BLYLLRCXAPBANCNFSM4KU4AAJQ> .

[Maaartinus]

I presume this won't address reference loops.

I'd bet, it won't. It's very hard to create immutable objects with loops as you need an object reference before the object gets created. It's doable, and there's a lengthy discussion in some Lombok issue, how this could be done, but it's damn complicated. Add inheritance and generics to the mix and enjoy....

Anyway, immutable reference loops would need a new sort-of-constructor creating all participants at once, not something as simple as a better wither.

[janrieke]

I like the idea, and it's indeed shorter. However, I'm not sure it's easier to comprehend for people not used to the new idiom.

I just though (only a few minutes, so this is neither very elaborated nor well-thought-out) about how such a feature could be designed when
a) we have to stick to the Java syntax, but
b) do not have to care much about how this could be implemented, because we could fix the whole with call chain once lombok runs. So here's my idea:

movie = movie.withDirector().withDob().withYear(1999);

The actual implementations of that (no-args) methods do nothing, they are just there to allow code completion and the correct return type of the last call. I'm quite sure you could achieve this using an inner helper class with a type parameter that is bound to the type of the first instance, Movie in this case.

When compiling, lombok could detect calls to those empty methods and replace them by the actual invocations, here:
movie.withDirector(movie.getDirector().withDob(movie.getDirector().getDob().withYear(1999)));

Sounds too weird? ;)

[Maaartinus]

@janrieke

I'm quite sure you could achieve this using an inner helper class

I guess, you'd need an inner class per field, but it could work. As the class is just imaginary, this doesn't matter much, but I'm not sure whether it helps understanding. Your syntax is nicer and shorter than the lambdas, but it's more magical and possibly not that flexible. It's

movie.withDirector().withDob().withYear(1999);

instead of

movie.withDirectorBy(d -> d.withDobBy(b -> b.withYear(1999)));

i.e., despite saving not many characters, you get a much simpler expression.

Sounds too weird? ;)

No, but I'm afraid, not doable.... your expression can occur in any class, and then you know nothing about Movie having withDirector() generated by Lombok. There may be no such method (and you should error out) or there may be a manually written method doing something completely different (and then you shouldn't touch it at all).

For this, we'd need something clearly indicating it's Lombok's stuff like

lombok.with().movie.withDirector().withDob().withYear(1999);

but without the arg-less methods actually existing, autocomplete wouldn't work and it's all rather ugly.

---

I'm trying to get inspiration from immerjs, immutable-js and other JS libraries, where this feature is called "deep updates". However, it seems to be too far (JS is way more flexible and the libraries try to provide much more).

[rzwitserloot]

Having this weird amalgamation of a classfile artifact (the args-list withDirector method) which literally must always just do nothing (I guess the impl will throw a RuntimeEx?), because its one and only purpose is for the CALLER, who must have lombok, to call it? – I don't want lombok to go there.

So far, we have never done a feature that double dips: Most lombok features require that lombok is used by the 'creator' of an API, not by the 'consumer' of an API, who can remain in complete blissful ignorance that lombok is being used by the API creator: @Getter requires that whomever makes a class uses this and has lombok on the path during builds, but after that, anybody who consumes this API just sees a getter. They don't need lombok. They don't need to know what lombok is. Nobody needs to mention lombok in their API docs. A few lombok features have this flipped around, primarily @ExtensionMethod, where an API consumer'd use it but is not reliant on the API creator having taken into account the existence of @EM as a concept when writing the API. But the double dip scenario where both consumer and creator must have lombok, and if either side does not the entire feature just does not make any sense whatsoever?

I don't think we should do that.

A secondary issue is this: Sure, looking at it as an academic example it looks nice. But when I'm writing code, I think in code. Specifically, if I see a chain of method invokes, I think about the return types of each individual part. Which doesn't work here; you shouldn't break down movie.withDirector().withDob().withYear(1999) in that way.

Unless.... of course we DO go in that direction. I think it is mandatory we do, as it solves both problems (the feature now no longer requires lombok to be used by API consumers, and you CAN break it down in this fashion if you want to):

public class Movie implements WithDirectorHaver<Movie> {
    DirectorLens<Movie> withDirector() { return new DirectorLens<>(this, getDirector()); }
    Movie withDirector(Director d) { /* usual impl */ }
}

@Generated
public interface WithDirectorHaver<T> {
    public T withDirector(Director d);
}

@RequiredArgsConstructor @Generated
public class DirectorLens<T extends WithDirectorHaver> {
    private final T root;
    private final Director director;

    public DobLens<T> withDob() {
        return new DobLens<>(root, director.getDob());
    }

    public T withDob(LocalDate dob) {
            return root.withDirector(director.withDob(dob));
    }
}

// same for WithDobHaver and DobLens

But there are a bunch of problems with this:

[1] Who makes DirectorHaver and co? It needs to clone all the with-methods of Director which requires resolution and in general it kinda makes more sense for Director to make it, but the XHaver is based on the field name and not the type (it is withDob(), not withLocalDate, so a withLocalDateHaver isn't useful.

[2] The sheer reams of code this would generate moves lombok into an area I don't want it to go: Whilst this feature is technically, like almost all lombok features, a syntax desugaring, in practice trying to explain this feature by showing how 2 lines of code explode into 8 classes and 1200 lines is not a good way to do it. But I don't like that; I want lombok features to be trivially to explain and/or explainable by showing a desugaring that one can still plausibly believe as something one would write by hand. (The notion of chainable builders, where each mandatory parameter is encoded as a separate interface, thus forcing a builder caller to go through each mandatory field in turn, gets denied as a lombok feature for the same reason. SOOO much code, nobody in their right mind would ever consider doing all that by hand).

@Maaartinus 's idea is also plausible as a lombok feature, but that one would probably rely on way too much magic. We can make:

package lombok;

public class Lombok { // this already exists today.
    public static <T> T deepWith(T in) { return in; }
}

and then have as lombok feature that it scans your code for calls to this thing and transform them. This:

Lombok.deepWith(movie).getDirector().getDob().withYear(1999);

turns into:

movie.withDirector(movie.getDir... you get the idea).

but I don't like this either; it's the lesser of the two things lombok does (cater to API consumers, instead of API builders; by its very nature that means you're doing things that are not idiomatic java and look bizarre), I foresee lots of maintenance issues (eclipse and intellij and such do need to figure out that the type of the above expression isn't LocalDate, it's Movie, and that'll probably require lots of finagling to make work and to maintain. It's also highly undiscoverable.

Perhaps the presence of a withDirectorBy method in the autocomplete of the movie reference does not immediately tell you: Hey, you can use this to deepWith! – but I think that's primarily an issue of a useful idiom that the java community simply has not discovered yet. Unlike all these alternatives, the withBy idiom is reasonable, today, without lombok: It's still a one-liner method if you want to add them to your non-lombok projects.

That is new (we normally use lombok to 'encode' existing common if unwieldy idioms, and ensure that lombok always generates the best form, even if you wouldn't do that if you were to write it by hand. Builders existed before lombok made the feature, for example). Here we are attempting to 'encode' an idiom we think is useful for this, but isn't (yet?) common. That is definnitely a reason to keep this one in the experimental box and see if we can use this to make the idiom more familiar to java coders.

Once it IS familiar, all these problems (lack of familiarity, lack of discovery) disappear.

[Maaartinus]

There's one more reason against

movie.withDirector().withDob().withYear(1999);

and relatives: You can't do things like

movie.withDirectorBy(d -> d.withFirstName('X').withDobBy(b -> b.withYear(1999)));

movie.withDirectorBy(d -> normalizeDirector(d));

and alike. By providing the nicer, lambda-less syntax, we'd lose the flexibility of functions.

[rzwitserloot]

I've chosen the following functional interfaces:

[obvious choices]

• long -> LongUnaryOperator
• int -> IntUnaryOperator
• double -> DoubleUnaryOperator
• object -> Function<? super T, ? extends T> - so not UnaryOperator because, hey, if you want to transform a field of type Number using a function that takes in any Object and returns an Integer, that should work.

not so obvious:

• boolean -> UnaryOperator<Boolean> – there is no BooleanOperator; going with the usual function seems odd here, as lowercase-b-boolean really isn't an object, so allowing a function that transforms any object into a boolean is weird, and clashes with what we do for other primitives. However, going with an IntUnaryOperator, 'casting' the boolean to a 1 or 0 value and 'casting' the response back seems to assign arbitrary truthy and falsy aspects to 0 and 1, which isn't idiomatic java so that doesn't feel right either.
• char, short, byte, float -> IntUnaryOperator (and for the float, DoubleUnaryOperator), which means your transformer is handed an int and not a char/short/byte/float, and whatever you return is then cast to char/short/byte/float to make it fit. Especially with char that isn't necessarily fantastic API, but I'm not sure offering a Function<? super Character, Character> is a better choice. These primitive types don't have an XUnaryOperator functional interface variant, and lombok does not want to add runtime dependencies, so I'm a bit stuck having to think of java.* functional interfaces that work well for these.

Are there compelling arguments to be consistent and use Function<? super X, ? extends X> for all of them, X being either the type of the field directly, or if primitive, the wrapper equivalent? If we do this (and we already do something like this with booleans), is it okay that if you return null, you just get a plain jane NPE, not specifically adorned by lombok with a clear message (though that is coming to JDK15 if memory serves, we'd get it automatically then).

[rzwitserloot]

@rspilker what's left is the docs. I'm leaving that, plus reviewing the work, to you.

[tlinkowski]

I can't wait for this feature! 😃

As to types of "not so obvious" functional interfaces:

• I don't think it matters that much (as long as Lombok doesn't become a runtime dependency) because I'm under an impression that this feature won't be used too often for char (let alone short or byte)
• certainly, choosing IntUnaryOperator for char aligns with the approach of JDK 8 (although I don't like how easy it becomes to confuse a character with a code point then)
• if I had to decide, I guess my slight preference would be to neglect performance and just have UnaryOperator<Character>, UnaryOperator<Byte>, etc. (especially knowing that work on Value Types for Java is in progress under Project Valhalla)

[Maaartinus]

if I had to decide, I guess my slight preference would be to neglect performance and just have UnaryOperator<Character>, UnaryOperator<Byte>, etc. (especially knowing that work on Value Types for Java is in progress under Project Valhalla).

This makes sense.... and one day we may get UnaryOperator<byte>, too. The future will probably render all current choices ridiculous, including all the special function interfaces like LongUnaryOperator (which then may or may not be declared as extends UnaryOperator<long>).

I wonder whether it's possible to account for the future in Lombok. Maybe there could be a configuration key, which currently would give you a single choice, namely doing what's described above. Anyone who gets a method dependent on any of the non-obvious choices would also get a warning, pointing to the doc describing the current state and recommending to add the configuration to silence the warning. The advantage is that the default could be changed in the future without breaking anyone (little gain for not much work). I'm afraid my explanation wasn't very good....

[elchtestAtBosch]

I only swept over the discussion in this ticket and think that it is already possible to get a builder from an existing object with object.toBuilder() and then modify that object arbitrarily and get a new one but I never tried that.

But what led me to this issue is that I am missing a way to clone a builder to implement the mother object pattern. https://reflectoring.io/objectmother-fluent-builder/

// builder for the mother
MyEntityBuilder mother = MyEntity.builder().value1(default1).value2(default2);

// derive variants of the mother object
MyEntity obj1 = mother.butWith().value2(nonDefault2).build();
MyEntity obj2 = mother.butWith().value1(nonDefault1).build();

The butWith() method would just return a copy of the builder.
Currently this is not possible since reusing the builder (without cloning it) and changing the defaults modifies the builder. Just creating a new builder often doesn't work since the default builders often use random values e.g.

public class MyEntityMother {
  // Returns an entity with all required fields initialized with dummy/random values.
  public static MyEntityBuilder minimalEntity() {
    return MyEntity.builder()
       .id(UUID.randomUuid())
       .requiredField("dummyValue");
  }
}

[rzwitserloot]

Imma go yell at @rspilker for not doing the review on this one, later.

[famod]

Are you guys going to finish this soon so that 1.18.24 can be relesed?

Asking because of #3134 which is a PITA given there is no edge p2 site (AFAICS) and using SNAPSHOTs in productive projects is rather "meh".

Thanks!

[rzwitserloot]

This weekend, @famod. With or without WithBy support, there'll be a release.

[famod]

Thanks a lot @rzwitserloot!

[rzwitserloot]

To be clear, the docs did not make it into v1.18.24, and I was a weekend late due to main paying job interference. But, v1.18.24 it's out since yesterday evening :)

[kozla13]

is this blocking 1.18.30 release ?
We need the Java 21 support without edge-SNAPSHOT

[rzwitserloot]

@kozla13 When you wrote that comment, 1.18.30 had been out for a while, the actually relevant issue (namely, 'JDK21 support') said so, and the site listed 1.18.30. Be nicer to open source developers: Do more than 5 seconds of checking before asking questions.

[kozla13]

@rzwitserloot
I apologize for the misunderstanding. I couldn't locate the version on Maven Central, so I mistakenly assumed it hadn't been released yet.