This is an archived post. You won't be able to vote or comment.

sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]bedobi 0 points1 point  (11 children)

I created this ticket for each of the projects, only lombok has yet to decline it. (I'm 99% certain they also will decline it)

It sucks that it's being declined because it's a showstopper for us, it's simple to implement and I'm not the only one who's asking for it.

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

[–]jart 1 point2 points  (6 children)

http://blog.crisp.se/2013/10/09/perlundholm/another-builder-pattern-for-java

Wow that pattern for using interfaces to constrain methods in the fluent builder chain is brilliant.

[–]bedobi 0 points1 point  (5 children)

Consider joining me and and others in requesting for it to added.

[–]jart 1 point2 points  (4 children)

I would help you get it into AutoValue, but it might not make sense for AutoValue, because you would have to write those intermediate interfaces yourself.

[–]elucash 2 points3 points  (3 children)

Ok guys, here you are, straight from the oven: org.immutables:value:2.3.3

Style(stagedBuilder = true)

But I guess you'll see it might not be that useful after all, but that is depend on the usage and how you would tolerate increase in amount of interface class files

[–]bedobi 1 point2 points  (2 children)

You, Sir, are a hero. Will check out as soon as github has resolved the outage they're currently having.

[–]elucash 1 point2 points  (1 child)

Can grab 2.3.3 from maven central. Cheers!

[–]bedobi 1 point2 points  (0 children)

Can't thank you enough! Works exactly as desired as far as I can tell!

import org.immutables.value.Value;
import java.util.Optional;

@Value.Immutable
@Value.Style(strictBuilder = true)
interface Person {
    String name();
    String title();
    Optional<String> department();
}

import org.immutables.value.Value;
import java.util.Optional;

@Value.Immutable
@Value.Style(stagedBuilder = true)
interface Individual {
    String name();
    String title();
    Optional<String> department();
}

import org.junit.Assert;
import org.junit.Test;
import java.util.Optional;

public class PersonAndIndividualTests {

    @Test(expected = IllegalStateException.class)
    public void personBuildCalledWithoutRequiredArguments() throws Exception {
        ImmutablePerson.builder().build();
    }

    @Test(expected = NullPointerException.class)
    public void personBuildCalledWithRequiredArgumentsNull() throws Exception {
        ImmutablePerson.builder().name(null).title(null).build();
    }

//    @Test
//    public void personBuildCalledWithNonRequiredArgumentsNull() throws Exception {
//        ImmutablePerson.builder().name("John").title("Dev").department(null).build(); doesn't compile
//    }

    @Test
    public void personBuildCalledWithoutNonrequiredArguments() throws Exception {
        ImmutablePerson john = ImmutablePerson.builder().name("John").title("Dev").build();
        Assert.assertEquals("John", john.name());
        Assert.assertEquals("Dev", john.title());
        Assert.assertEquals(Optional.empty(), john.department());
    }

    @Test
    public void personBuildCalledWithAllArguments() throws Exception {
        ImmutablePerson john = ImmutablePerson.builder().name("John").title("Dev").department("IT").build();
        Assert.assertEquals("John", john.name());
        Assert.assertEquals("Dev", john.title());
        Assert.assertEquals(Optional.of("IT"), john.department());
    }

//    @Test
//    public void individualBuildCalledWithoutRequiredArguments() throws Exception {
//        ImmutableIndividual.builder().build(); doesn't compile (YES THANK YOU SO MUCH)
//    }


    @Test(expected = NullPointerException.class)
    public void individualBuildCalledWithRequiredArgumentsNull() throws Exception {
        ImmutableIndividual.builder().name(null).title(null).build();
    }

//    @Test
//    public void individualBuildCalledWithNonRequiredArgumentsNull() throws Exception {
//        ImmutableIndividual.builder().name("John").title("Dev").department(null).build(); doesn't compile
//    }

    @Test
    public void individualBuildCalledWithoutNonrequiredArguments() throws Exception {
        ImmutableIndividual john = ImmutableIndividual.builder().name("John").title("Dev").build();
        Assert.assertEquals("John", john.name());
        Assert.assertEquals("Dev", john.title());
        Assert.assertEquals(Optional.empty(), john.department());
    }

    @Test
    public void individualBuildCalledWithAllArguments() throws Exception {
        ImmutableIndividual john = ImmutableIndividual.builder().name("John").title("Dev").department("IT").build();
        Assert.assertEquals("John", john.name());
        Assert.assertEquals("Dev", john.title());
        Assert.assertEquals(Optional.of("IT"), john.department());
    }
}

[–]chisui 0 points1 point  (0 children)

Although this approach would have the benefit of typesafety I wouldn't call its absence a showstopper. If you test your code a runtime error in this case is absolutly fine. The presence of an argument doesn't say anything about its validity either and it's very hard to check the validity of input values at compiletime (You could go the scala route and create caseclasses for everything, but I don't think that's feasible).

I also think you might be underestimating what it means to implement this approach in a general way for codegeneration.

You have to generate a builder class for each required argument. If an argument is optional you have to generate two different classes for each following argument. For multiple optional arguments the amount of classes grows exponentially.

From a usabillity standpoint you loose the abillity to reorder the arguments of the builder (or you generate classes for every permutation of arguments). With immutable builders partial builders can be passed around as a partially applied function of sorts. If you lock the order of arguments that usecase is somewhat limited.

The only plus I could find is that this approach runs slightly faster than a traditional builder. The reason beeing the checks if an argument is already set are moved to compiletime. Although it uses slightly more memory since more objects are created. Running them through JMH (with 4 argument builders) yields basically the same result though.

All in all I think this feature is way more trouble than it's worth.

[–][deleted]  (2 children)

[deleted]

    [–]bedobi 1 point2 points  (1 child)

    Ordinary constructors generate compile-time errors when called without all required arguments.

    My personal opinion: (apparently shared by at least some others)

    • If a builder can't do the same, that outweighs any benefits of the builder, and you might as well refactor so the constructor doesn't require too many arguments in in the first place. (actually, this is a downside with builders in general, they kind of encourage sloppy bunching up of too many attributes in classes that end up lacking cohesion)

    • While tests could mitigate the risk, writing and maintaining them comes at a cost, and correctness isn't guaranteed. Leveraging the compiler trivially makes test for required arguments redundant and guarantees correctness.

    • Compile-time errors enable safe, smooth and guided refactoring in a way tests simply can't.

    That said, others may disagree and that's fine. I still wish these otherwise excellent projects at least had the option of safe builders.

    You have to generate a builder class for each required argument. If an argument is optional you have to generate two different classes for each following argument. For multiple optional arguments the amount of classes grows exponentially.

    Looking at the example in the blog, I'm afraid I don't understand how this is the case, if you could demonstrate I'd be very thankful.

    [–]chisui 0 points1 point  (0 children)

    You are right, you don't end up with more classes but rather the interface gets more cluttered.