Configuration concept and implementation

I created an API to load, save and read user configuration in my application. I've a Configuration interface which provides the basic methods to read, save and read configuration data and then in the application I've the implementation JsonConfiguration.

I tried to keep the Configuration interface as generic as possible. So I didn't make any assumption about how the settings are stored, it can be plain text as well as something else.

This is my Configuration interface:

```
public interface Configuration {
/**
* Called when the configuration should load the settings
*
* @param inputStream In this stream you can found the settings of
* the user.
* An implementation can decide what is inside this
* stream.
* Example: If an implementation
* uses JSON to save and read settings, it should
* assume that this stream contains a valid json.
* If it doesn't, it is allowed to throw
* any exception since it's not
* their job to convert the stream to your
* format.
* Do not close the stream. You don't own it.
*/
void load(@NotNull final InputStream inputStream);

/**
* Called when the configuration should save ALL the settings
*
* @param outputStream Write in this stream the settings of the
* user in the form of the implementation.
* Do not close the stream. You don't own it.
*
* @throws IOException Throw if something went wrong during the
* save process
*/
void save(@NotNull final OutputStream outputStream) throws IOException;

/**
* @param name The setting name
* @return The setting as integer (if it's an integer)
* or NumberFormatException if not
*/
@NotNull
OptionalInt getAsInt(@NotNull final Stri

• As I said, the concepts save/load and read are mixed together in one interface, make sense separate them and implement a Factory to load and save settings?

Wells, Java's Properties provide similar load/store functionality as well, so I think this is fine.

• I want to remove the throws IOException from save because it's not consistent with load which doesn't throw IOException.

As mentioned by yourself in the lengthy comments, a custom Exception class foremost may be more helpful to wrap underlying reasons such as IOException (or SQLException, UnknownHostException etc.). The question then becomes whether you want to stick with checked Exception classes or not.

If you prefer checked Exception classes, then I think both save/load methods should throw it for consistency. As helpfully explained here, here and here, checked Exceptions may be used to indicate to the method's callers that they should be able to reasonably recover from these scenarios. For your implementation, I think a checked, custom Exception class has its merits for users of your library to explicitly handle cases where their configuration could not be initialized properly.

• Can I improve the load of JsonConfigurationSection? The ifs seems okay, but maybe could be improved.

You may want to consider the fail-fast approach here:

for (final Map.Entry entry : root.entrySet()) {
    final JsonElement value = entry.getValue();
    if (value.isJsonNull()) {
        throw new IllegalArgumentException("null is not a valid parameter");
    } else if (value.isJsonArray()) {
        throw new UnsupportedOperationException("Arrays not supported yet");
    } else if (value.isJsonObject()) {
        throw new UnsupportedOperationException("Objects not supported yet");
    } else if (value.isJsonPrimitive()) {
        // note: inlined entry.getKey() and value.getAsJsonPrimitive()
        values.put(entry.getKey(), value.getAsJsonPrimitive().getAsString());
    }
}

It may not be apparent now, but once your library gains new features, e.g. for array/object support, you can gradually remove the checks from the top and append the implementations (or preferably method calls to the new features) below.

• Any comment? I've read how other languages/framework does it but...

When you are converting your Optional instance to one of the OptionalInt/OptionalLong, you can rely on the map().orElse() chained methods to better convey the conversion:

return optional.map(v -> OptionalInt.of(Integer.parseInt(v)))
                .orElse(OptionalInt.empty());

I also don't think you really need an Optional case... do you really need a tri-state configuration choice, true, false and null? Wouldn't it be easier to just say true or false?

You also mentioned...


Since I don't make any assumption in the Configuration interface, the concept of "sections" exists only in the implementation...

Do you intend to support a tree hierarchy of configuration in future versions? That may be worth pondering about...

StackExchange Code Review Q#97238, answer score: 2