1

So I am building a utility JAR for an EAR and I have a class which provides static methods which need an XML string. I am embedding an XML file as a resource in my JAR and I read it at class initialization time in a static block:

static{
    try {
        base = IOUtils.toString(CartSessionFactory.class.getResourceAsStream("/createcartTemplate.xml"));
        if(base==null){
            throw new Exception("createcartTemplate.xml not found as a resource");
        }
        //check the file correctness
        //[...]
            }
        }
    } catch (Exception e) {
        e.printStackTrace();
    }
}

A fellow coder told me he was horrified at that design but could not explain clearly why (but recommanded making a Singleton instead). It seems to work though. Is it bad design and why?

pHneutre
  • 1,091
  • 3
  • 12
  • 29
  • What happens if `base == null`? As in, after this block is executed? What happens the first time somebody tries to use `base`? – Andy Turner Feb 06 '17 at 14:51
  • Not related with resource loading, but one of the things you should care about is that `if`: Java doesn't have the `goto` feature in order to avoid spaghetti code. If you intentionally throw an exception which gets catched by an outer `try` (and ignored too) it has the same behaviour as a `goto`, which is a strongly discouraged practice. – BackSlash Feb 06 '17 at 14:54
  • "but recommanded making a Singleton instead" Could he give a good reason for this? – Andy Turner Feb 06 '17 at 14:55
  • @AndyTurner if base is null there is something very wrong so I'd simply expect the exception to display in the logs. It would appear at class initialization and would mean the application is broken. Not sure what is the meaning of your question. About the Singleton I guess it's just good practice according to the guy. – pHneutre Feb 06 '17 at 15:48
  • @BackSlash could you please elaborate on the goto behaviour? I don't see it. To me, an exception is thrown & catched and the stacktrace is displayed, end of the story. Where's the spaghetti? – pHneutre Feb 06 '17 at 15:51
  • 1
    With regard to singletons, I mean [What is so bad about singletons](http://stackoverflow.com/q/137975/3788176) – Andy Turner Feb 06 '17 at 16:06
  • @AndyTurner I wish I could propagate the Exception but it doesn't seem to be possible in an initialization block. So maybe that's one of the problems with my design :) how would you proceed to load a resource? Is creating a documented initialization method the only way? It does not seem satisfying. – pHneutre Feb 06 '17 at 16:21
  • 1
    @pHCito This is the goto behaviour: You have an `if`, which will throw an exception, catched in the `catch` block, printed and ignored. This means `if ( base is null ) goto -> exception block`. You could just print something in the if and use an `else` block to avoid execution of other code which should not execute if `base` is `null`, without throwing an exception like that. [Here](http://stackoverflow.com/questions/29358806/should-i-use-exception-to-simulate-a-goto-statement-in-java) there is also a related question – BackSlash Feb 06 '17 at 16:33

1 Answers1

0

I would inject this dependecy into the class and not hard code it into it. Testing this module is not as easy as if you would inject the resource into it.

thopaw
  • 3,796
  • 2
  • 17
  • 24
  • Can you be more specific about the dependency injection? How would you inject a dependancy to a file in the class? – pHneutre Feb 06 '17 at 15:29