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

Recursive Fields Generator #258

Open
jsuskalo-kingland opened this issue Feb 26, 2020 · 4 comments
Open

Recursive Fields Generator #258

jsuskalo-kingland opened this issue Feb 26, 2020 · 4 comments
Labels

Comments

@jsuskalo-kingland
Copy link
Contributor

Right now there's a convenient way to generate data based on the fields of a class with the @From(Fields.class) annotation placed on the argument to a function, but this has a problem that if not all the fields have registered generators, then it will fail to generate the value.
It would be very helpful to developers working with nested data structures to be able to have something like @From(RecursiveFields.class) which would then generate each field using a fields generator. This would greatly simplify working with nested POJOs and remove a lot of boilerplate that looks like so:

public class SomeDataClassGen extends Generator<SomeDataClass> {
    public SomeDataClassGen() {
        super(SomeDataClass.class);
    }

    public SomeDataClass generate(SourceOfRandomness r, GenerationStatus s) {
        return gen().fieldsOf(SomeDataClass.class).generate(r, s);
    }
}

This also makes it easier to deal with the POJOs in a case where in most situations you can just use a base generator from fields, but later introduce a generator specific to the class, but without breaking older tests or significantly changing the semantics of the generators for said old tests.

@pholser
Copy link
Owner

pholser commented Feb 26, 2020

@jsuskalo Thanks for this suggestion. TBH, when I first put together junit-quickcheck I wasn't sure whether developers would take to Fields/Ctor or whether they'd do a lot of rolling custom generators.

Instead of going after gen(field) for every field of a class, RecursiveFields might instead say, look, every field value gets generated by Fields. Should it still honor other registered generators that might be able to generate values of a given field's type? Should it honor configuration annotations on those fields also?

@jsuskalo-kingland
Copy link
Contributor Author

My initial thought was that the entire tree would be generated strictly as if by the Fields generator, although I think it would be good if it respected registered generators and took into account configuration annotations, passing them down to each generator which can take them down the whole tree.
However, I would love to see some discussion on what the best semantics are for if it should respect other generators, etc.

@pholser
Copy link
Owner

pholser commented Feb 27, 2020

@jsuskalo Maybe RecursiveFields can check gen(field) such that if no registered generator is found, it swaps in an instance of RecursiveFields. Thinking maybe these gen() methods ought to be returning Optional<Generator<?>>....?

@jsuskalo-kingland
Copy link
Contributor Author

Yeah, that makes a lot of sense to me, however I'm not sure I'm a fan of changing the gen methods to give an optional generator, since that would break existing callers.
I'd suggest adding some new genIfExists() methods which return the optional, that way existing callers don't break, but we can still use it to implement fetching a generator if it exists, and otherwise substituting a RecursiveFields generator.

So it sounds to me like the semantics should be as follows:

  • For each field, attempt to find a generator, and if it can't, use a RecursiveFields
  • For each field, configure the fields with any configurations applied to RecursiveFields
  • For any field which uses the RecursiveFields generator, pass all configurations to it

I also don't think that we need extra instances of RecursiveFields for each field to be generated, most likely you can just call this.generate() for each field you want to use RecursiveFields on, that way you don't have to worry about passing the configurations on to the new instances, and it would reduce memory requirements compared to having a new instance per field.

So to fetch a particular generator for a given field, we might be able to just use something like this:

Generator<?> fieldGenerator = gen().typeIfExists(field.getType()).orElse(this);
if (fieldGenerator != this) {
    // ... configure the generator ...
}
Object value = fieldGenerator.generate(random, status);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants