Add fluent combinator API about jqwik HOT 8 CLOSED

I like the idea. Just playing with potential APIs.

Consider those domain classes

public static class Person {
	private final String name;
	private final int age;
	public Person(String name, int age) {
		this.name = name;
		this.age = age;
	}
	@Override
	public String toString() {
		return String.format("%s (%s)", name, age);
	}
}

public static class PersonBuilder {
	private String name = "A name";
	private int age = 42;
	public PersonBuilder withName(String name) {
		this.name = name;
		return this;
	}
	public PersonBuilder withAge(int age) {
		this.age = age;
		return this;
	}
	public Person build() {
		return new Person(name, age);
	}
}

The test could look like this:

@Property
void fluentCombinators(@ForAll("people") Person aPerson) {
	System.out.println(aPerson);
}

@Provide
Arbitrary<Person> people() {
	BuilderCombinator<PersonBuilder> builderCombinator = Combinators.withBuilder(new PersonBuilder());
	Arbitrary<String> names = Arbitraries.strings().alpha().ofLength(10);
	Arbitrary<Integer> ages = Arbitraries.integers().between(0, 130);
	return builderCombinator
			   .use(names).in(PersonBuilder::withName)
			   .use(ages).in(PersonBuilder::withAge)
			   .build(PersonBuilder::build);
}

What do you think?

There's a potential problem with the builder instance being reused but one can solve that with lazy generation.

from jqwik.

That would work well for my use case. I'm slightly turned off by the separated use/in methods and the implications that has for the implementation, but I could live with it. :)

I was feeling pretty proud of myself with how general my original proposal was. Hypothetically, it could be useful for things other than working with builders. But, it's hard to synthesize another good use case for chaining combinators together.

from jqwik.

@jdmarble Could you elaborate the negative implications of use/in?

I introduced use/in because it reads a bit more fluent (for my taste) and it's a bit closer to 'Combinators.combine(...).as(...)'. And the generality that was in your approach is still preserved.

from jqwik.

Yes, I can elaborate. Consider the return type of BuilderCombinator.use:

• If it is BuilderCombinator then the argument to use must be stored as state for future calls. In my opinion, superfluous state is at least a small negative. How do we enforce that in is called after use and only one in is called before a use and the argument to use has a type that matches the argument of use etc... It seems there is no type safe (compile time checked) way to implement these constraints.
• If it is some other type, say IntermediateBuilderCombinator, then it can be made to be type safe, but that is one extra class to implement, maintain, and understand. I'll admit that this is a small cost, but it should be weighed against the benefit of fluency.

Now I see that it is nearly as general as my approach. One thing that tripped me up was that the name (BuilderCombinator) makes it seem that it is only applicable to the builder pattern. This is, currently, my only use case, so I'm okay with that. In fact, I think that is a massive advantage for user understanding if it is the only thing it should be used for.

What if we want to combine more than two arbitraries? It would be difficult to do this in a type safe way without having IntermediateBuilderCombinator1, IntermediateBuilderCombinator2, etc... With an "extended map", it would look like:

    public <U, A1> Arbitrary<U> map(Combinators.F2<T, A1, U> mapper, Arbitrary<A1> a1) {
        return Combinators.combine(this, a1).as(mapper);
    }

    public <U, A1, A2> Arbitrary<U> map(Combinators.F3<T, A1, A2, U> mapper, Arbitrary<A1> a1, Arbitrary<A2> a2) {
        return Combinators.combine(this, a1, a2).as(mapper);
    }

    // ... and so on ...

Here's another (contrived) use case for this idea that isn't exactly a builder:

        return Combinators.withBuilder(Calendar.getInstance())
            
            .use(Arbitraries.integers().between(1990, 2000))
            .in((calendar, year) -> calendar.set(Calendar.YEAR, year))
            
            .use(Arbitraries.integers().between(5, 15))
            .in((calendar, weekOfYear) -> calendar.set(Calendar.WEEK_OF_YEAR, weekOfYear))
            
            .use(Arbitraries.integers().between(2, 5))
            .in((calendar, dayOfWeek) -> calendar.set(Calendar.DAY_OF_WEEK, dayOfWeek))
            
            .build(calendar -> calendar);

from jqwik.

@jdmarble Thanks for the valuable feedback.

A statically typed split between use and in requires an additional class indeed. This is a bit more complex to implement. The reason I prefer this API is the symmetry to Combinators.combine(..).as(..) which works the same way. I've come to prefer this style of fluent API in recent years since reasonable line breaking is a bit more obvious than breaking lines inbetween a parameter list.

With a single parameter the implementation cost is not very high either. Here's my prototypical implementation (I chose a new entrance class BuildCombinator):

public class BuilderCombinator<B> {
	private Arbitrary<B> builder;
	public static <B> BuilderCombinator<B> with(Supplier<B> builderSupplier) {
		return new BuilderCombinator<>(builderSupplier);
	}
	private BuilderCombinator(Supplier<B> builder) {
		this(Arbitraries.create(builder));
	}
	private BuilderCombinator(Arbitrary<B> delegate) {
		this.builder = delegate;
	}
	public <T> CombinableBuilder<B, T> use(Arbitrary<T> arbitrary) {
		return new CombinableBuilder<>(builder, arbitrary);
	}
	public <T> Arbitrary<T> build(Function<B, T> buildFunction) {
		return builder.map(buildFunction);
	}

	public static class CombinableBuilder<B, T> {
		private final Arbitrary<B> builder;
		private final Arbitrary<T> arbitrary;
		private CombinableBuilder(Arbitrary<B> builder, Arbitrary<T> arbitrary) {
			this.builder = builder;
			this.arbitrary = arbitrary;
		}
		public <C> BuilderCombinator<C> in(Combinators.F2<B, T, C> toFunction) {
			Arbitrary<C> arbitraryOfC = 
					builder.flatMap(b -> arbitrary.map(t -> toFunction.apply(b, t)));
			return new BuilderCombinator<>(arbitraryOfC);
		}
	}
}

Having a use with 2,3,.. parameters would require an additional CombinableBuilder per number of params.

As for naming, I'm a bit torn between BuilderCombinator, ArbitraryBuilder and something with "Fluent" in it.

My suggestion: I include something along the lines sketched above as experimental feature, and you - and others - can give me feedback about how it feels in real usage.

from jqwik.

@jdmarble Thanks for the valuable feedback.

A statically typed split between use and in requires an additional class indeed. This is a bit more complex to implement. The reason I prefer this API is the symmetry to Combinators.combine(..).as(..) which works the same way. I've come to prefer this style of fluent API in recent years since reasonable line breaking is a bit more obvious than breaking lines inbetween a parameter list.

With a single parameter the implementation cost is not very high either. Here's my prototypical implementation (I chose a new entrance class BuildCombinator):

public class BuilderCombinator<B> {
	private Arbitrary<B> builder;
	public static <B> BuilderCombinator<B> with(Supplier<B> builderSupplier) {
		return new BuilderCombinator<>(builderSupplier);
	}
	private BuilderCombinator(Supplier<B> builder) {
		this(Arbitraries.create(builder));
	}
	private BuilderCombinator(Arbitrary<B> delegate) {
		this.builder = delegate;
	}
	public <T> CombinableBuilder<B, T> use(Arbitrary<T> arbitrary) {
		return new CombinableBuilder<>(builder, arbitrary);
	}
	public <T> Arbitrary<T> build(Function<B, T> buildFunction) {
		return builder.map(buildFunction);
	}

	public static class CombinableBuilder<B, T> {
		private final Arbitrary<B> builder;
		private final Arbitrary<T> arbitrary;
		private CombinableBuilder(Arbitrary<B> builder, Arbitrary<T> arbitrary) {
			this.builder = builder;
			this.arbitrary = arbitrary;
		}
		public <C> BuilderCombinator<C> in(Combinators.F2<B, T, C> toFunction) {
			Arbitrary<C> arbitraryOfC = 
					builder.flatMap(b -> arbitrary.map(t -> toFunction.apply(b, t)));
			return new BuilderCombinator<>(arbitraryOfC);
		}
	}
}

Having a use with 2,3,.. parameters would require an additional CombinableBuilder per number of params.

As for naming, I'm a bit torn between BuilderCombinator, ArbitraryBuilder and something with "Fluent" in it.

My suggestion: I include something along the lines sketched above as experimental feature, and you - and others - can give me feedback about how it feels in real usage.

from jqwik.

For the moment, I'll go with Combinators.withBuilder(...).use(...).in(...).build(...).

Implemented in a5348f2

Available in latest 1.1.1-SNAPSHOT

from jqwik.

Will be available in 1.1.1 (deployment ongoing). Feel free to reopen if something should be improved.

from jqwik.