Enforce setting of Optional fields

[pgr0ss]

What do you think about an option to force consumers of a builder to explicitly set optional fields. For example:

@FreeBuilder
public interface Person {
  Optional<String> getFavoriteColor();
}

Person person = new Person.Builder()
    .setFavoriteColor(Optional.empty())
    .build();

Person person = new Person.Builder()
    .build(); // throws an Exception since favoriteColor is not set

This would ensure that all attribute are accounted for, and prevent empty optionals from creeping in accidentally.

[j-baker]

Hey! You can do this already if you like, for example:

@FreeBuilder
public interface Person {
    Optional<String> getFavoriteColor();

    class Builder extends Person_Builder {
        @Override
        public Person build() {
            Person person = super.build();
            Preconditions.checkState(getFavoriteColor().isPresent(), "You must set the favorite color");
            return person;
        }
    }
}

does this accomplish what you need?

[hdurer]

The problem here is that Optional has two uses in Java: a) as safer alternative to null, b) to represent data that may not have been provided. As I understand it, the OP wanted a) and have FB enforce that it is set in the builder.

2016-09-13 9:44 GMT+01:00 James Baker [email protected]:

Hey! You can do this already if you like, for example:

@FreeBuilder public interface Person { Optional<String> getFavoriteColor();

class Builder extends Person_Builder {
    @Override
    public Person build() {
        Person person = this.build();
        Preconditions.checkState(getFavoriteColor().isPresent(), "You must set the favorite color");
        return person;
    }
}

}

does this accomplish what you need?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/FreeBuilder/issues/191#issuecomment-246615391, or mute the thread https://github.com/notifications/unsubscribe-auth/AACp3V2yCF3KeIVfR5Nz4aHueHyq3v6Iks5qpmJagaJpZM4J7KeW .

[alicederyn]

In other words, to not default Optional fields to empty. You can already do that manually by adding a missingX field, and overridding setX/merge/build to maintain it. In terms of adding explicit support to FreeBuilder, I would rather support general pluggable behaviour (issue #14) and have this be one plug-in we provide. Will leave this feature request open for voting.

[pgr0ss]

@chrisalice Can you elaborate on your missingX field workaround? I don't think I understand it. Thanks.

[j-baker]

so the simplest case would be something like (in the builder) something like:

@FreeBuilder
public interface Person {
    Optional<String> getFavoriteColor();

    class Builder extends Person_Builder {
        boolean isFavoriteColorSet = false;

        @Override
        public Builder setFavoriteColor(String favoriteColor) {
            isFavoriteColorSet = true;
            return super.setFavoriteColor(favoriteColor);
        }

        @Override
        public Builder clearFavoriteColor(String favoriteColor) {
            isFavoriteColorSet = true; // if you want clearing the favorite color to set it to false, you'd need to override the optional and nullable setters as well.
            return super.clearFavoriteColor();
        }

        @Override
        public Person build() {
            Preconditions.checkState(isFavoriteColorSet, "must set the favorite color to something");
            return super.build();
        }
    }
}

which is pretty unpleasant (looking at JavaUtilOptionalSourceTest.java i'm not sure you need to do anything with merge).

[pgr0ss]

Yeah, that's not ideal, especially when we have multiple Optional fields on a class. I guess I'll wait for pluggable behavior, then. Thanks.

[alicederyn]

You'll need to explicitly set isFavoriteColorSet in the mergeFrom(Builder) method, too (otherwise it will always end up true even if neither builder had it set beforehand).

On Tue, 13 Sep 2016, 22:04 Paul Gross, [email protected] wrote:

Yeah, that's not ideal, especially when we have multiple Optional fields on a class. I guess I'll wait for pluggable behavior, then. Thanks.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/google/FreeBuilder/issues/191#issuecomment-246823755, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7xqSPSIa0w3buLAWbTDFsuTOBMavvfks5qpw_IgaJpZM4J7KeW .