Use UncheckedIOException rather than IllegalStateException for consistency

[agentgt]

In io.avaje.config.Config we have

public class Config {

  private static final Configuration data = CoreConfiguration.initialise();
...
}

Any static method called on Config will cause initialization of data. Luckily and I assume this is why @rbygrave wrote it that way is that every static method in Config happens to access data.

However another developer could easily make the mistake of adding a static method that does not need data. Perhaps a utility method used by the builder or whatever like concatPath or somethign.

Thus I recommend the canonical holder pattern of a private static inner class:

public class Config {

  private static class Holder {
    static final Configuration data = CoreConfiguration.initialise();
  }
...
}

All the static config methods would now do Holder.data for data.

[SentryMan]

I've no objections to this

[agentgt]

Also this:

avaje-config/avaje-config/src/main/java/io/avaje/config/InitialLoadContext.java

Line 102 in 6915e81

throw new IllegalStateException(e);

Should throw either UncheckedIOException or ignored (as in the file no longer exists).

EDIT that whole code block looks like duplication of stuff that Configuration.builder should do or know about:

  InputStream resource(String resourcePath, InitialLoader.Source source) {
    InputStream is = null;
    if (source == InitialLoader.Source.RESOURCE) {
      is = resourceStream(resourcePath);
      if (is != null) {
        loadedResources.add("resource:" + resourcePath);
      }
    } else {
      File file = new File(resourcePath);
      if (file.exists()) {
        try {
          is = new FileInputStream(file);
          loadedResources.add("file:" + resourcePath);
          loadedFiles.add(file);
        } catch (FileNotFoundException e) {
          throw new IllegalStateException(e);
        }
      }
    }
    return is;
  }

EDIT whoops I put this in the wrong place

[rbygrave]

Holder ... However another developer could easily make the mistake of adding a static method that does not need data.

I'd say that is extremely unlikely. The design is such that Config has only the one job and I don't see that design changing. We get no real benefit from adding a Holder class per se. I don't think we need to make such a change.

Should throw either UncheckedIOException or ignored (as in the file no longer exists).

Well I don't think it should be ignored, I don't see a strong argument that ignoring that FileNotFoundException is the right thing to do.

It is certainly an unexpected state that a resource file exists and then doesn't when it is read. Throwing UncheckedIOException seems ok but IllegalStateException also seems ok given it has detected an unexpected and illegal state. I'll have a closer look there.

[rbygrave]

So yes lets change to use UncheckedIOException for better consistency.

I'll rename this issue to reflect this change. I don't see the desire for the Holder change but it you want to continue to push for that add some more comments here and lets see.