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

[Proposal] Replace DefaultValues-class with Provider. #178

Open
marcus-talbot42 opened this issue Nov 15, 2022 · 3 comments · May be fixed by #192
Open

[Proposal] Replace DefaultValues-class with Provider. #178

marcus-talbot42 opened this issue Nov 15, 2022 · 3 comments · May be fixed by #192

Comments

@marcus-talbot42
Copy link
Contributor

The DefaultValues-class is rather rigid, and extensions of the class are inherently invasive.

I propose to replace the DefaultValues-class with a Provider-interface, and an implementation DefaultValuesProvider. Please find the example implementation below.

@FunctionalInterface
public interface Provider<S, T> {

    T get(S source);

}

An implementation for the Provider-interface, that would offer roughly the same functionality as the DefaultValues-class, could look like this:

public class DefaultValueProvider<S, T extends S> implements Provider<Class<S>, T> {

    private static final Map<Class<?>, ?> DEFAULT_VALUES = Map.ofEntries(
            Map.entry(boolean.class, false),
            Map.entry(byte.class, (byte) 0),
            Map.entry(short.class, (short) 0),
            Map.entry(int.class, 0),
            Map.entry(long.class, 0L),
            Map.entry(float.class, 0.0F),
            Map.entry(double.class, 0.0),
            Map.entry(List.class, Collections.emptyList()),
            Map.entry(Set.class, Collections.emptySet()),
            Map.entry(Map.class, Collections.emptyMap()),
            Map.entry(Optional.class, Optional.empty())
    );

    @Override
    public T get(Class<S> source) {
        if (source == null)
            return null;
        return (T) DEFAULT_VALUES.get(source);
    }
}

It could also be implemented as a record:

public record DefaultValuesProviderRecord<S, T extends S>(Map<Object, Object> defaultValues) implements Provider<S, T> {

    private static final Map<Object, Object> DEFAULT_VALUES = Map.ofEntries(
            Map.entry(boolean.class, false),
            Map.entry(byte.class, (byte) 0),
            Map.entry(short.class, (short) 0),
            Map.entry(int.class, 0),
            Map.entry(long.class, 0L),
            Map.entry(float.class, 0.0F),
            Map.entry(double.class, 0.0),
            Map.entry(List.class, Collections.emptyList()),
            Map.entry(Set.class, Collections.emptySet()),
            Map.entry(Map.class, Collections.emptyMap()),
            Map.entry(Optional.class, Optional.empty())
    );

    public DefaultValuesProviderRecord() {
        this(DEFAULT_VALUES);
    }

    @Override
    public Map<Object, Object> defaultValues() {
        return Collections.unmodifiableMap(this.defaultValues);
    }

    @Override
    public T get(S source) {
        return (T) this.defaultValues().get(source);
    }
}

Aside from the benefits for extensibility of the Provider itself, this will also positively impact testing, and the composition of Form - and Result-classes.

I also propose to allow, not one, but any number of providers for any given mapping, by modifying the Configuration to include a Collection of Provider-objects. This would allow users a greater degree of control over the mapping of their objects, and would be especially easy to use, due to the Provider-interface being a functional interface.

@berkayerten
Copy link

I'd like to pick up this issue

@marcus-talbot42
Copy link
Contributor Author

That'd be great, thank you!

@FdHerrera
Copy link

Hello,

I implemented the changes suggested here in this PR
Would be great to here your thoughts whenever you have a chance.

Ty!

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

Successfully merging a pull request may close this issue.

3 participants