• Post Reply Bookmark Topic Watch Topic
  • New Topic

is it ok to pass a new FileInputStream on a method? Is it necessary to close it or how to close it?  RSS feed

 
Matt Taylor
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys,

I have a method that accepts an InputStream. When calling that method, is it ok to pass a new instance of Input Stream?

For example:

The method signature is:


is it ok to invoke it with the following:



in this example, close() is not invoked from FileInputStream. Will it cause memory leak or problems?

or will it be better if I do this:

 
Stephan van Hulst
Saloon Keeper
Posts: 7976
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My policy is that unless you're passing a stream to a constructor of a class that wraps the stream, the method that created the stream is responsible for closing it. Don't close Streams when they're passed as method arguments, close them in the calling method using try-with-resources:

When you're passing a stream to a constructor of a wrapper, you do this:

You can then call it like this:
 
Matt Taylor
Ranch Hand
Posts: 72
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
^thanks for the response. This is not a constructor but an ordinary method.

So does it mean, on my example:



is not efficient / not a good practice?

Instead I should do this:



Can you please verify?
 
Stephan van Hulst
Saloon Keeper
Posts: 7976
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
you should always close resources. Creating a file stream and not closing it is bad practice, and may even cause problems if your application needs to open the same file multiple times in a short span of time, or if you open a lot of files and the operating system won't let you because you have too many open handles.

You should close resources using the syntax I showed you above. try-with-resources automatically closes resources, you don't have to do it in a finally clause anymore.

As a matter of fact, the way you close the stream in the finally-block is inappropriate, because it may throw an exception, and you should never throw exceptions in finally-blocks.

If you don't explicitly need a FileInputStream, you should use the Files class to open a stream using a Path.

Lastly, you should catch specific exception types. Catching Exception is considered bad practice.
 
Alex Lieb
Ranch Hand
Posts: 61
3
Java Netbeans IDE Notepad
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you open an InputStream, you need to be able to close it later. If you pass a new FileInputStream object to another method without opening it, the method which created the InputStream has no references to the object which it can use to close it, and you always want to close streams in the same method that creates them.

So of the two options you initially suggested, this is better than just passing a new FileInputStream as a parameter:



But that has no way to catch IO exceptions, and it should, so you could do something like this, and it would be good:



*Or* if you're using Java 7 or 8 you could use a try-with-resources like Stephen suggested; when you enter one, it instantiates any IO objects you tell it to instantiate, then when it exits, it closes them without you having to tell it to close them. Either way though, the important points here are:

1) You should always close your streams.
2) You should always close your streams *in the same method that created them*
3) In order to adhere to 1 and 2, your stream must remain available to the method which creates it
 
Stephan van Hulst
Saloon Keeper
Posts: 7976
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You don't necessarily have to catch an IOException, you can also propagate it to the caller. You are responsible however for not letting any exceptions leave the finally-block. So before Java 7, this is how you properly used a stream, assuming you could propagate the IOException:
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rob Spoor, gives another solution here.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!