Java – Is it better not to .close() inside API method body when passed a Reader or InputStream

javaresourcesstream-processing

There are plenty of similar questions that have been answered. Example here. However, they all have both the Reader and the InputStream within the same scope or method body, and so suggest to close the last in the chain (Reader) instead of the first (InputStream).

So for my particular case, I am building an API, and want the users to give a choice of which input source to use. They need flexibility. For this I typically follow this type of template:

public class Handler {
  public void handle(BufferedReader in) {
    in.lines().forEach(System.out::println);
  }

  public void handle(InputStream in) {
    handle(new BufferedReader(new InputStreamReader(in)));
  }

  public void handle(Class anchor,String resource) throws IOException {
    try (InputStream in = anchor.getResourceAsStream(resource)) {
            handle(in);
    }
  }

  public void handle(String resource) throws IOException {
    try (InputStream in = ClassLoader.getSystemResourceAsStream(resource)) {
            handle(in);
    }
  }

  public void handle(Path path) throws IOException {
    try (InputStream in = Files.newInputStream(path)) {
            handle(in);
    }
  }

  public void handle(File file) throws IOException {
    try (InputStream in = Files.newInputStream(file.toPath())) {
            handle(in);
    }
  }

  public static void main(String args[]) throws NoSuchMethodException, IOException { 
    Handler h = new Handler();
    h.handle(Handler.class,"/readme.txt");
  }
}

I think it has following benefits:

  • Only the owner of he resource can close it. In the case of reader/input, the benefit is not so clear. However, if it would have been writer/output, then this would have allowed me to have several such functions operate consecutively after each other on the same resource. I think that makes sense as an API.
  • Simple to understand: if you pass/own the resource, you have the responsibility of closing it.
  • If you pass along a resource name (String), it is clear that the api will have to take care of both opening and closing the resource.

So there is a difference with the other postings, both in situation, and how to answer it:

  • Here: if Reader or InputStream gets passed as a parameter to a method of an API, then do not .close() on it inside the API method body.
  • Others: if within the same scope, InputStream is chained with a Reader, then always close the last in the chain (the Reader).

I think this makes sense, but is this a correct way of handling this?

Best Answer

I would argue that neither is the best solution.

When you are dealing with a Stream or Reader of some kind then it is implied this is for reading Input or writing Output to some externally available data or resources. This resource may or may not be associated to an operating system file thus in calling a .close() method on it means you could also potentially affect something at the OS level.

Generally speaking, if a method needs to obtain a Stream or Reader of some kind then the safest bet is to properly handle instantiating the Stream or Reader from a resource(s), handling exceptions that could crop up, and finalizing the use of the resource(s) before leaving the scope of the method call.

If the file or resource is large or a potentially infinite data stream then it probably makes sense for the scope of a Stream or Reader to live longer than the scope of a method. If the Stream or Reader lifecycle is tied to the lifecycle of a Class instance, then the appropriate option is have the Stream or Reader as a private scoped property of the Class such that methods in that class instance can utilize it and depend on it. You need to be careful here about cleaning up and closing your resources though because when that class instance is garbage collected then it will also garbage collect your Stream or Reader. I don't believe that every JVM implementation guarantees the finalize() method will be called on garbage collect but I may be wrong here.

The bottom line is that it is considered BAD PRACTICE in Java to open a Stream or Reader and pass it as an argument to a method. The only time I would say this is appropriate is if you have a very very large method and you wish to break up the code into small helper methods, but even then these should be private scoped methods and should be documented. I certainly wouldn't want to see a method that accepts this as a parameter attempt to close it on behalf of the method below it on the stack. From a code review perspective as well, I would want to see where resource handlers are being initialized, and ultimately clearly see where they are also guaranteed to be finalized and closed out.

Related Topic