3
\$\begingroup\$

I have this following piece of code:

@Component
public class ManifestReader {

    private final ServletContext servletContext;
    private Properties p;

    @Autowired
    public ManifestReader(final ServletContext servletContext) {
        this.servletContext = servletContext;
    }

    public Properties getManifest() throws IOException {
        if (p == null) {
            p = new Properties();
            p.load(servletContext.getResourceAsStream("/META-INF/MANIFEST.MF"));
        }
        return p;
    }
}

What I want to do it is just, to load the file, and once it is loaded, it can be injected to all the classes that need that properties file. Like here:

@RestController
@RequestMapping(value = "/info")
public class InfoEndPoint {

    private final ManifestReader manifestReader;

    @Autowired
    public InfoEndPoint(final ManifestReader manifestReader) {
        this.manifestReader = manifestReader;
    }

    @RequestMapping(method = RequestMethod.GET)
    public Info get(final HttpServletRequest httpServletRequest) throws IOException {
        final App app = fillApp(httpServletRequest, manifestReader.getManifest());
        final Info info = new Info();
        info.setApp(app);
        return info;
    }
    ....
}

As you can see, everytime I have to call to getManifest I throw an IOException. I don't really want that since I just need to load it once, and then it can be reused without throwing any exception. So, my idea is that you could call to the getManifest method without having to check for any exception.

My first idea was to load the file in the constructor of the class, but I do not know if that is a good practice for Spring. Any clue?

\$\endgroup\$
  • \$\begingroup\$ My code is working, but I am asking how to improve it \$\endgroup\$ – Manuelarte May 25 '16 at 11:44
2
\$\begingroup\$

My first idea was to load the file in the constructor of the class, but I do not know if that is a good practice for Spring.

It takes the lazy-loading out of the lazy-loader, but then you could ask yourself what the remaining uses of the class are.

As you can see, everytime I have to call to getManifest I throw an IOException.

There are two main ways around this:

1. Use UncheckedIOException to wrap IOException:

    public Properties getManifest() {
      if (p == null) {
        p = new Properties();
        try {
          p.load(servletContext.getResourceAsStream("/META-INF/MANIFEST.MF"));
        } catch (IOException ex) {
          throw new UncheckedIOException(ex);
        }
      }
      return p;
    }

This isn't my preferred approach, because all this does is evade the radar, basically. If you're in a pinch, though, this will do the trick. Mind: UncheckedIOException is not a type of IOException.

(On another note: you probably need to mark p as volatile!)

2. Move the loading to an initialization method:

    public Properties getManifest() {
      return p;
    }

    @PostConstruct
    public void init() throws IOException {
      p = new Properties();
      p.load(servletContext.getResourceAsStream("/META-INF/MANIFEST.MF"));
    }

This approach has the added benefit of being thread-safe: no need to fence access to p, and no risk of having a bean in an inoperable state, or filling the log with errors.

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

You can use try catch() block in getManifest() method.

public Properties getManifest() {
    if (p == null) {
        try {
           p = new Properties();
           p.load(servletContext.getResourceAsStream("/META-INF/MANIFEST.MF"));
        } catch(IOException ioex) {
           ioex.printStackTrace();
        }
    }
    return p;
}
\$\endgroup\$
  • 3
    \$\begingroup\$ I wouldn't exactly call this an improvement, as you end up with no Properties when the exception gets thrown. As the getManifest method would only be called once in the application, I'd consider this a fatal error, which would be somewhat hidden when the loading actually fails. \$\endgroup\$ – Friek Aug 18 '17 at 22:22
  • \$\begingroup\$ That's right, it would be better to thrown unchecked exception. If loading manifest fails, we should stop the application. \$\endgroup\$ – KirkoR Aug 21 '17 at 11:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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