5
\$\begingroup\$

I have a factory that loads configuration from an xml file, and uses the configuration to be able to create objects of an appropriate type with appropriate settings. My application does a number of different tasks, and I don't want a broken or missing xml file to interfere with the other tasks the application does.

The factory is a Guice-provided dependency, however, if I make the constructor throw an exception, then I will just get a ProvisionException, which could cause unpredictable results or crash the application, neither of which are acceptable.

Currently, I have the factory swallow the exception on creation, and then throw it when the make method is called. Alternatively, I have a makeUnchecked method, if you call the ready() method first. Are there any problems with this approach, or is there potentially a better way to do it?

SSCCE:

public class FooFactory {
    private Exception ex = null;

    @Inject
    public FooFactory(@FooSettings String filename) {
        loadSettings(filename);
    }

    public void loadSettings(String filename) {
        try {
            // stuff
            this.ex = null;
        } catch(Exception e) {
            this.ex = e;
        }
    }

    public Foo make(InputObj input) throws Exception {
        if(ex != null) throw ex;
        makeUnchecked(input);
    }

    public Foo makeUnchecked(InputObj input) {
        if (ex != null) throw new IllegalStateException(ex);
        // factory stuff
    }

    public boolean ready() {
        return ex == null;
    }
}
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

My application does a number of different tasks, and I don't want a broken or missing xml file to interfere with the other tasks the application does.

If you don't need the XML file configuration, why do you have it? How can you continue without it?

How you answer this conceptual question has a lot to do with how you implement the code logic using exceptions.

\$\endgroup\$
0
\$\begingroup\$

Your approach will work, but I would suggest an alternative. Remove the makeUnchecked and ready methods. Do not have constructor load settings. Have a static boolean variable, say, areSettingsLoaded to indicate whether settings have been loaded. Set it to false initially. In the make method add thread-safe code to check the value of areSettingsLoaded. If false call loadsettings. Then set the boolean var to true. This approach will have the following advantages:

  1. The class gets constructed faster.
  2. The Exception will not be hidden.
  3. Exception handling logic is much simpler.
\$\endgroup\$
2
  • \$\begingroup\$ Would you still have make throw a checked exception? \$\endgroup\$
    – durron597
    Aug 28, 2013 at 13:15
  • \$\begingroup\$ A disadvantage is that it makes testing harder since the static variable acts as a global state which you have to reset before every test. \$\endgroup\$
    – palacsint
    Oct 27, 2013 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.