Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should I close a StringReader?

Tags:

I use a StringReader to turn a string into something I can upload to an SFTP server (it takes a stream). Is there any point in closing that StringReader afterwards? As far as I can see in the source it just sets the string to null...

I could just do it, but since the close method is marked as throwing an IOException and all I have to wrap it in a try catch and the code just ends up looking a lot more horrible than it perhaps needs to be.

like image 588
Svish Avatar asked May 25 '11 09:05

Svish


3 Answers

If you know you're dealing with a StringReader that you'll be throwing away, I don't see any reason to close it. I can't imagine any reason you'd be holding a reference to it after you'd close it, so there's no real benefit to the string being set to null for garbage collection. If you were creating a method that takes a Reader then it might make sense to close it since you don't know the underlying type.

like image 85
WhiteFang34 Avatar answered Sep 19 '22 09:09

WhiteFang34


It does more than that. If I may quote the JavaDoc:

/**
 * Closes the stream and releases any system resources associated with
 * it. Once the stream has been closed, further read(),
 * ready(), mark(), or reset() invocations will throw an IOException.
 * Closing a previously closed stream has no effect.
 */

So yes, you should close that reader. Not for the sake of resources but for the sake of good style and programmers that may follow you. You don't know where this instance will be passed to and what someone else will try to do with it. Someday you might also choose to change the interface and accept any Reader implementation in which case you might deal with a Reader that requires a call to close() to free resources.

So it is good style to prevent the further (possibly wrong) use of this instance once you're done with it. And since it doesn't hurt, it will only prevent possible errors in the future.

Edit: Since you say, that your close() method is declaring an exception it might throw I would say that you need to call close() since StringReader.close() does not throw an exception. However, Reader.close() does. So you already allow other implementations of Reader and so you must close it since you can't know what implementations of Reader you'll eventually get. If we are talking about three lines of code that never leave that scope, declare your variable StringReader and call close anyway (in that case without exception handling).

like image 22
Stephan Avatar answered Sep 20 '22 09:09

Stephan


Though strictly not necessary, because StringReader only holds onto a String, as a matter of good form its always a good idea to close all Readers anyway. Today your code might be using a StringReader but if you changed it to another Reader that really needs to be closed, your code w/out the close would be wrong while your w/ close would be fine.

like image 34
mP. Avatar answered Sep 21 '22 09:09

mP.