4

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?

4
  • 2
    Sorry for not giving you the answer you want, but as for "where to close()", I leave that to Lombok by annotating the InputStream variable with @CleanUp. I ♥ Lombok – Jaroslav Záruba Feb 4 '16 at 22:16
  • @JaroslavZáruba Yeah - looks promising, but I have a headache trying to read through the website. I am sure if the website gets cleaned up a little, it would have more users in a jiffy. The home page is a video, and it has a giant lady bug - kidding me ... to follow trends, they should have a website like "lombok.io". – YoYo Feb 4 '16 at 22:23
  • 1
    Generally speaking, Readers and InputStreams should be treated the same, as basically their only difference is characters versus bytes. – Louis Wasserman Feb 4 '16 at 22:38
  • @LouisWasserman There is no "invisible magic". Lombok simply closes the resources at the end of the block they were declared in... Which you can confirm using the de-lombok function. – Jaroslav Záruba Feb 5 '16 at 6:51
1

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.

3
  • 1
    If it is bad practice, then why are there such examples in the core Java API? Agreed, not exactly an example of best coding practices. I did notice that by allowing to accept a Stream, my code becomes more manageable. Specifically I am able to keep streaming logic with the Objects that should know (themselves). In that case, I guess, it makes sense not to close the stream within such methods. I guess this might also be in-line with your 'long-lived' part of the answer. I see that I am often doing the same for other resources, like a Database Connection ... – YoYo Mar 19 '16 at 18:30
  • 2
    @YoYo Well not everything in the API was a good idea either. I think Java Date and Calendar APIs live as a testament to some poor design choices in the standard API. Database connections may come from a pool where close has been overridden to simply return to the pool. Even still I would hope you are not passing database connections around as method parameters. – maple_shaft Mar 19 '16 at 18:44
  • The overhead of reacquiring a connection from a resource is very expensive. Of course I would be passing around a database connection. Just imagine if the resource would happen to be a connection pool. Reusing the same DB connection might be for very trivial things: 1./ Creating or preparing the table (truncating/creating partition); 2./ Insert the object; 3./ possibly resulting in elements of a Collection to be inserted; 4./ finalizing the table (gathering analytics). Not only that ... I cannot pass along a transactional context if I do not keep the connection open. – YoYo Mar 19 '16 at 19:57

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.