Add Dynamic Accessors

[ctrimble]

This PR adds methods for getting and setting properties by name, either in one of the declared fields or in the additional properties map. This code generates getter methods like this:

protected final static Object NOT_FOUND_VALUE = new Object();

protected Object declaredPropertyOrNotFound(String name, Object notFoundValue) {
    switch (name) {
        case "someField":
            return getSomeField();
        // more cases here for fields on this schema.
        default:
            // call to super.getField$(name, notFoundValue) if extending another schema.
            return notFoundValue;
    }
}

public<T >T get(String name) {
    Object value = declaredPropertyOrNotFound(name, NOT_FOUND_VALUE);
    if (NOT_FOUND_VALUE!= value) {
        return ((T) value);
    } else {
        // IllegalArgumentException here if no additional properties.
        return ((T) getAdditionalProperties().get(name));
    }
}

and setter methods like this:

protected boolean declaredProperty(String name, Object value) {
    switch (name) {
        case "someField":
            if (value instanceof String) {
                setSomeField(((String) value));
            } else {
                throw new IllegalArgumentException(("property \"someField\" is of type \"java.lang.String\", but got "+ value.getClass().toString()));
            }
            return true;
        // more cases here for fields in this schema.
        default:
            // call to super.setField$(name, value) if extending another schema.
            return false; 
    }
}
public void set(String name, Object value) {
   if (!declaredProperty(name, value)) {
        // IllegalArgumentException here if no additional properties.
        getAdditionalProperties().put(name, ((Object) value));
    }
}

when generateBuilders == true, then a builder method is also generated, with the signature

public THIS_TYPE with( String property, Object value )

when the target == 1.6, if/elseif blocks are used, instead of String switch statements.

This feature can be turned off by setting includeDynamicAccessors to false.

[ctrimble]

This change, and the others related to filtering out static fields, should probably come in as a separate PR, since this is technically broken on the 0.4.X line of development.

[ronnyroeller]

+1

[cminardi]

Great feature, I currently have use cases in which this would definitely be useful. Have you considered expanding this feature allowing get$() and set$() to work on json pointers, or paths to a specific field?
For example: get$("foo.bar") would execute get$("foo").get$("bar").
This would come in handy when you have to access nested objects, and although it brings a little complexity to the get$ and set$ methods, it increases their power remarkably.

[ctrimble]

@cminardi I had not planned on getting into pathing with this change, since that would probably lead to jsonpath support. That seems outside the bounds for this project and would probably be better facilitated by an external tool. It looks like https://github.com/nebhale/JsonPath might do what you want.

[ctrimble]

@joelittlejohn Is the intention of includeAccessors to not generate any methods at all and only output fields? If so, it seems like this PR should respect that flag.

[joelittlejohn]

Yes exactly that, public fields.
On 18 Nov 2015 6:26 pm, "Christian Trimble" notifications@github.com
wrote:

@joelittlejohn https://github.com/joelittlejohn Is the intention of
includeAccessors to not generate any methods at all and only output
fields? If so, it seems like this PR should respect that flag.

—
Reply to this email directly or view it on GitHub
#386 (comment)
.

[ctrimble]

Great. I will add in checks for that flag and some more testing, since I am generating methods now and the test suite is passing.

[ctrimble]

The $ character has been removed from the setter and getter methods. That character seemed to imply that some kind of EL was being evaluated.

[ctrimble]

This should probably be declaredPropertyOrNotFound. The current name implies that the provided value is returned for null.

[ctrimble]

Once #454 is on master, this change this change should be rebased and squashed. Otherwise, it is complete.

[ctrimble]

When squashing this PR, bring all of the changes like this into a single commit, before the actual feature.

[joelittlejohn]

Nice work, thanks for continuing with this.

Is 1.6 support something you need for your own project @ctrimble? It's nearly three years since public updates stopped for that version of Java so I'm not sure we still need to support it. I'll leave that up to you though (it seems to add a lot of code and complexity to this PR). The population of users that use this feature, and the population of users that are still on Java 6 are two sets that may never intersect :)

The last thing we need to here is put this behind a config flag. People can get very particular about the code generated and there's now a lot added when this feature is active.

[ctrimble]

@joelittlejohn I didn't know if the android people were all ready to move up to 1.7, so I left in support. If you are comfortable with moving the version up, then perhaps we should just drop the 1.6 support, so the project doesn't have to lug it around.

In either case, I would like to get the target version exposed to the rules and test suite. I could really use some Java8 features and I know the android programmers are not ready to make that move.

For instance, methods like this would be very handy for me:

  public T with(Consumer<T> withCalls) {
    withCalls.accept(this);
    return this;
  }

As for adding a flag, that is not a problem. includeByNameAccessors seems like it would be consistent with the other flags in the project.

[joelittlejohn]

@ctrimble Very good point, I hadn't considered Android. I also agree having a target version will be useful for Java 8.

That flag sounds fine (maybe includeDynamicAccessors?), it's a bit cryptic but the docs will help 😄

[ctrimble]

I like includeDynamicAccessors better. I will add that in and start preparing for a merge. I could really use input on the PR I opened specifically for the target option.

[ctrimble]

Added the includeDynamicAccessors property. It seems strange that this option also adds builder methods.

[joelittlejohn]

Took me a while to understand that comment :) I don't think that's a problem.

[ctrimble]

I have rebased this on master. Some naming alignment and clean up should be done before a merge happens.

[ctrimble]

This rule should be called DynamicPropertiesRule, to align naming with the flag.

[ctrimble]

This should be DynamicPropertiesRuleTest, to align name with flag.

[ctrimble]

@joelittlejohn I think this PR is ready to go, unless you see anything that should be cleaned up.