Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify updating immutable members when referencing the previous value #20

Closed
runfalk opened this issue Jan 11, 2021 · 7 comments
Closed
Labels
enhancement New feature or request question Further information is requested

Comments

@runfalk
Copy link

runfalk commented Jan 11, 2021

Hey there! I've been using your library a bit and think it's a nice improvement for records. When working with nested immutable types I've found some thing a bit cumbersome. Imagine that you have the following records:

@RecordBuilder
public record Context(int id, Counter count) implements ContextBuilder.With {}

@RecordBuilder
public record Counter(int count) implements CounterBuilder.With {}

If we want to increment the count in a context we need to do this:

var ctx = new Context(1, new Counter(0));

// I know withCounter can be used as well
var newCtx = ctx.with().counter(ctx.counter().with().counter(ctx.counter().count() + 1).build());

We need to repeat the ctx.counter() bit three times unless we want to bring in a temporary variable. It would be nice if RecordBuilder provided something that simplified mutation of members that are record or other immutable types like queues.
Ideally something that allows us to add arbitrary helper methods to the builder would be nice, but maybe that's difficult to do?

Another idea would be to generate variants of the setters in the builder that takes a Function<T, T>. That allows the user to mutate the inner types. The example above would become something like:

var ctx = new Context(1, new Counter(0));
var newCtx = ctx.with().counter(ctr -> ctr.with().count(c -> c + 1).build());

The drawback is of course that it could conflict with records that have members with a type of Function.

@Randgalt
Copy link
Owner

Randgalt commented Jan 11, 2021

If we had an idea of where the Java language was going with this we could try to simulate it. For your case, this would work, though I'm not sure how complex the annotation processing code would be. If a record component is a record that implements RecordBuilder's with, additional with methods would be added. E.g. Context's With would get:

default Context withCount(UnaryOperator<Integer> operator) {...}

Then, you could write:

var ctx = new Context(1, new Counter(0));
var newCtx = ctx.withCount(count -> count + 1);

I'm a bit loathe to add too much specialized functionality. Is this a general case or an edge case?

UPDATE

Actually, the only safe thing to do is add:

default Context withCount(Consumer<CounterBuilder> consumer) {...}

Then you could do:

var ctx = new Context(1, new Counter(0));
var newCtx = ctx.withCount(counter -> counter.count() + 1);

Not quite as clean but much more doable. Otherwise, RecordBuilder would have to explode all the Record components on the nested record. Unless you can think of something else.

@runfalk
Copy link
Author

runfalk commented Jan 11, 2021

Thank you for the quick reply! I like your second idea. It seems like a really nice convenience function to have anyway.

I do have a second more generic use case as well. Today I wrote an ImmutableQueue<E>:

var q1 = new ImmutableQueue<Integer>();
var q2 = q1.add(1).add(2).add(3); // Each add returns a new ImmutableQueue instance

q1.size() // 0
q2.size() // 3

This queue is a member of one of my records:

@RecordBuilder
public record Worker(int id, ImmutableQueue<Integer> tasks) {}

// Currently I need to do this (I'm using the with() syntax since I actually only have access to the builder where I'm mutating the record in my current code)
var worker = new Worker(1, ImmutableQueue.of());
var newWorker = worker.with().tasks(worker.tasks().add(1));
var newerWorker = newWorker.with().tasks(newWorker.tasks().discard());

// It would have been nicer to do something like
var newWorker = worker.with().tasks(t -> t.add(1));
var newerWorker = newWorker.with().tasks(t -> t.discard());

// Or ideally
var newWorker = worker.with().addedTask(1);
var newerWorker = newWorker.with().removedTask();

I appreciate that records are still a bit up in the air. This is getting into complex territory, but would it be possible to have something like:

@RecordBuilder
@RecordBuilder.IncludeExtras({WorkerBuilderExtensions.class})
public record Worker(
    int id,
    ImmutableQueue<Task> tasks) {}

interface WorkerBuilderExtensions extends WorkerBuilder /* Though I assume this is a class and not an interface */ {
    default WorkerBuilder addedTask(Integer i) {
        return tasks(tasks().add(i));
    }

    default WorkerBuilder removedTask(Integer i) {
        return tasks(tasks().remove());
    }
}

I haven't done any pre-processors so I don't even know if something like this is possible.

@Randgalt
Copy link
Owner

It occurs to me that you can do this manually. You can add the specialized method to Context yourself ala:

    @RecordBuilder
    public record Context(int id, Counter count) implements ContextBuilder.With {
        public Context withCount(Consumer<CounterBuilder> consumer) {
            var builder = count.with();
            consumer.accept(builder);
            return withCount(builder.build());
        }
    }

Now you can write:

        var newCtx2 = ctx.withCount(c -> c.count(c.count() + 1));

Given that this is specialized and likely not used often my feeling is: just do this. What do you think?

@runfalk
Copy link
Author

runfalk commented Jan 11, 2021

That's kind of OK, but in my code I actually don't have access to the object. I have a cache that can be accessed using ID. So something like this.

interface Identifieable {
    int id();
}

@RecordBuilder
public record Worker(int id, ImmutableQueue<Integer> tasks) implements Identifiable {}

var cache = new Cache();
cache.add(new Worker(1, ImmutableQueue.of());
cache.update(1, mutator -> mutator.tasks(...));

Of course I can change the API for the update method but it would be nicer if we could somehow override/add methods to the builder itself. I appreciate that you may want to keep things simple though.

@Randgalt
Copy link
Owner

Randgalt commented Jan 11, 2021

You could as easily write a Utility class to do it. I don't know what that Cache class looks like but with the Context/Counter example:

public class Helper {
    public static Context withCount(Context context, Consumer<CounterBuilder> consumer) {
        var builder = context.count().with();
        consumer.accept(builder);
        return context.withCount(builder.build());
    }
}

Then

import static Helper.withCount;

var newCtx2 = withCount(ctx, c -> c.count(c.count() + 1));

@Randgalt Randgalt added enhancement New feature or request question Further information is requested labels Jan 12, 2021
@Randgalt
Copy link
Owner

Note - a similar discussion is going on here: #21

@Randgalt
Copy link
Owner

Randgalt commented Feb 7, 2021

Closing this in favor of the discussion here: #21

@Randgalt Randgalt closed this as completed Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants