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

Rework the configuration system #174

Open
marcus-talbot42 opened this issue Nov 8, 2022 · 1 comment
Open

Rework the configuration system #174

marcus-talbot42 opened this issue Nov 8, 2022 · 1 comment

Comments

@marcus-talbot42
Copy link
Contributor

The current configuration system feels a little bit clunky. Many fields could be moved to a common abstract superclass, and various methods are hard-coded to return null or throw an Exception.

I propose to research and possibly implement, depending on the results, a fairly fundamental (and breaking) change, that would, in my opinion, improve the configuration system.

Concretely I propose the following:

  • Do away with the CoreConfiguration and OverrideConfiguration classes, but retain the Configuration interface.
  • A paradigm shift, from considering configurations to be semi-mutable, to making them immutable data-carriers (records). Consider the following example:
public record BeanMapperConfiguration(/* The necessary fields */) implements Configuration {
    public BeanMapperConfiguration(final Builder builder) {
        this(/* The necessary fields, recovered from the builder */);
    }

    public static class Builder {
        // Mutable fields mirroring those in the BeanMapperConfiguration
        public Builder(final Configuration configuration) {
            // Populate fields based on the given configuration, just like the current OverrideConfiguration would.
        }

        public BeanMapperConfiguration build() {
            return new BeanMapperConfiguration(this);
        }
    }
}

Note that this does allow the user to create custom configuration-implementations still, as we retained the Configuration interface. However, I do recommend trimming the Configuration interface, to suit the immutable data-carrier paradigm, by removing the mutator-methods.

  • As the current BeanMapperBuilder is more of a ConfigurationBuilder, I propose scrapping the BeanMapperBuilder entirely, and replacing it with a BeanMapperFactory, looking something like this:
public final class BeanMapperFactory {
    public static BeanMapper createBeanMapperWithDefaultConfiguration() {
        return new BeanMapper(new BeanMapperConfiguration.Builder().build());
    }

    public static BeanMapper createBeanMapperWithModifiedConfiguration(final Configuration configuration) {
        return new BeanMapper(configuration);
    }
}

The example could also be implemented as a Singleton, for thread-safety.

@marcus-talbot42
Copy link
Contributor Author

Feedback from @robert-bor: Consider shorter method-names in BeanMapperFactory, to make them more pleasant to read.

E.g.

  • createBeanMapperWithDefaultConfiguration -> withDefaultConfig
  • createBeanMapperWithModifiedConfiguration -> withModifiedConfig

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

When branches are created from issues, their pull requests are automatically linked.

1 participant