Closing Files that Weren't Successfully Opened

Discussion in 'Java' started by KevinSimonson, Mar 14, 2011.

  1. Almost from as far back in my software education as I can remember,
    people who would know have told me that it's good practice to close
    one's files when one is done with them, so when I write programs that
    open files I always try to do that. But I think I've found some grey
    area that makes this policy a little ambiguous.

    Whenever I write a program that uses the classes <File> and <Scanner>
    to read in lines of text from a file, when I go to compile it my
    compiler declares, "unreported exception
    java.io.FileNotFoundException; must be caught or declared to be
    thrown". And whenever I write a program that uses the classes
    <FileWriter>, <BufferedWriter>, and <PrintWriter> to write lines of
    text to a file, when I go to compile it my compiler gives a similar
    declaration: "unreported exception java.io.IOException; must be caught
    or declared to be thrown".

    So, being pretty obedient to what the compiler tells me to do when I
    can figure out what that is, I always add a <try-catch> block to
    handle the mentioned exceptions.

    My question is this. If I declare a variable <scnnr> to be of type
    <Scanner>, and try to open it with the statement <scnnr = new
    Scanner( new File( "Xy.Txt"))>, and a <FileNotFoundException> gets
    thrown, should I still do a <scnnr.close()>, say in the <catch>
    block? And similarly, if I declare <prntWrtr> to be of type
    <PrintWriter> and try to open it with the statement <prntWrtr = new
    PrintWriter( new BufferedWriter( new FileWriter( "Xy.Txt")))>, and an
    <IOException> gets thown, should I still do a <prntWrtr.close()>, also
    probably in the <catch> block? Or in each such situation can I
    conclude that since an exception occurred while I was attempting to
    open the respective file, the variables will still each both be
    <null>, and therefore I don't have to do anything?

    Kevin Simonson
     
    KevinSimonson, Mar 14, 2011
    #1
    1. Advertising

  2. Mon, 14 Mar 2011 15:27:26 -0700 (PDT), /KevinSimonson/:

    > My question is this. If I declare a variable <scnnr> to be of type
    > <Scanner>, and try to open it with the statement <scnnr = new
    > Scanner( new File( "Xy.Txt"))>, and a <FileNotFoundException> gets
    > thrown, should I still do a <scnnr.close()>, say in the <catch>
    > block? And similarly, if I declare <prntWrtr> to be of type
    > <PrintWriter> and try to open it with the statement <prntWrtr =
    > new PrintWriter( new BufferedWriter( new FileWriter( "Xy.Txt")))>,
    > and an <IOException> gets thown, should I still do a
    > <prntWrtr.close()>, also probably in the <catch> block? Or in each
    > such situation can I conclude that since an exception occurred
    > while I was attempting to open the respective file, the variables
    > will still each both be <null>, and therefore I don't have to do
    > anything?


    As you've noticed, if the object constructor throws, you'll get no
    object reference to invoke a method on. It is responsibility of the
    construction code to clean any unreachable resources allocated prior
    throwing the exception.

    --
    Stanimir
     
    Stanimir Stamenkov, Mar 14, 2011
    #2
    1. Advertising

  3. Stanimir Stamenkov <> wrote:
    > Mon, 14 Mar 2011 15:27:26 -0700 (PDT), /KevinSimonson/:
    >> My question is this. If I declare a variable <scnnr> to be of type
    >> <Scanner>, and try to open it with the statement <scnnr = new
    >> Scanner( new File( "Xy.Txt"))>, and a <FileNotFoundException> gets
    >> thrown, should I still do a <scnnr.close()>, say in the <catch>
    >> block? And similarly, if I declare <prntWrtr> to be of type
    >> <PrintWriter> and try to open it with the statement <prntWrtr =
    >> new PrintWriter( new BufferedWriter( new FileWriter( "Xy.Txt")))>,
    >> and an <IOException> gets thown, should I still do a
    >> <prntWrtr.close()>, also probably in the <catch> block? Or in each
    >> such situation can I conclude that since an exception occurred
    >> while I was attempting to open the respective file, the variables
    >> will still each both be <null>, and therefore I don't have to do
    >> anything?

    > As you've noticed, if the object constructor throws, you'll get no
    > object reference to invoke a method on. It is responsibility of the
    > construction code to clean any unreachable resources allocated prior
    > throwing the exception.


    This responsibility covers only stuff that has been allocated within
    that constructor that throws. If Scanner were to throw, then it would
    not clean up the open File passed to it.
    To solve it correctly, you(the OP) do your construction work stepwise:

    File file; Scanner scnnr;
    try {
    file = new File ("Xy.Txt");
    scnnr= new Scanner ( file );
    } catch (FileNotFoundException e) {
    if (file != null) { file.close(); }
    ... other cleanup
    } catch (SomeOtherException e) {
    // ditto
    } ...

    Yes, that isn't as "beautiful" as putting several "new"s into a single
    line, but as you saw, that beauty came with a price (possible ressource-
    leakage), that you seem not to like (and correctly so).
     
    Andreas Leitgeb, Mar 14, 2011
    #3
  4. Andreas Leitgeb <> wrote:
    > Stanimir Stamenkov <> wrote:
    >> Mon, 14 Mar 2011 15:27:26 -0700 (PDT), /KevinSimonson/:
    >>> My question is this. If I declare a variable <scnnr> to be of type
    >>> <Scanner>, and try to open it with the statement <scnnr = new
    >>> Scanner( new File( "Xy.Txt"))>, and a <FileNotFoundException> gets
    >>> thrown, should I still do a <scnnr.close()>, say in the <catch>
    >>> block? And similarly, if I declare <prntWrtr> to be of type
    >>> <PrintWriter> and try to open it with the statement <prntWrtr =
    >>> new PrintWriter( new BufferedWriter( new FileWriter( "Xy.Txt")))>,
    >>> and an <IOException> gets thown, should I still do a
    >>> <prntWrtr.close()>, also probably in the <catch> block? Or in each
    >>> such situation can I conclude that since an exception occurred
    >>> while I was attempting to open the respective file, the variables
    >>> will still each both be <null>, and therefore I don't have to do
    >>> anything?

    >> As you've noticed, if the object constructor throws, you'll get no
    >> object reference to invoke a method on. It is responsibility of the
    >> construction code to clean any unreachable resources allocated prior
    >> throwing the exception.

    > This responsibility covers only stuff that has been allocated within
    > that constructor that throws. If Scanner were to throw, then it would
    > not clean up the open File passed to it.
    > To solve it correctly, you(the OP) do your construction work stepwise:
    > File file; Scanner scnnr;
    > try {
    > file = new File ("Xy.Txt");
    > scnnr= new Scanner ( file );
    > } catch (FileNotFoundException e) {
    > if (file != null) { file.close(); }
    > ... other cleanup
    > } catch (SomeOtherException e) {
    > // ditto
    > } ...
    > Yes, that isn't as "beautiful" as putting several "new"s into a single
    > line, but as you saw, that beauty came with a price (possible ressource-
    > leakage), that you seem not to like (and correctly so).


    On re-read it seems I have to clarify a bit: That thing with separate
    construction seems not all that relevant to the File+Scanner combo.

    It's more of a general thing, and would principially apply on any chain
    of constructions, where any inner step really allocates resources, that
    ask for explicit release (which File doesn't).
    Maybe it applies better to the Writer-chain, but I'm now too lazy to look
    that up for documented special cases of convenience-cleanup.
     
    Andreas Leitgeb, Mar 15, 2011
    #4
  5. KevinSimonson

    Dagon Guest

    KevinSimonson <> wrote:
    > it's good practice to close one's files when one is done with them


    True.

    >So, being pretty obedient to what the compiler tells me to do when I
    >can figure out what that is, I always add a <try-catch> block to
    >handle the mentioned exceptions.


    You need to apply some amount of thought in addition to obedience. The
    compiler says you must catch _or_ declare the exception. Adding a try-catch
    block without knowing why is not advised.

    >My question is this. If I declare a variable <scnnr> to be of type
    ><Scanner>, and try to open it with the statement <scnnr = new
    >Scanner( new File( "Xy.Txt"))>, and a <FileNotFoundException> gets
    >thrown, should I still do a <scnnr.close()>, say in the <catch>
    >block?


    No. scnnr has not been initialized - an exception was thrown in trying to
    instantiate a Scanner to assign to scnnr. scnnr.close() will throw
    NullPointerException.

    > open it with the statement <prntWrtr = new PrintWriter(
    > new BufferedWriter( new FileWriter( "Xy.Txt")))>, and an
    ><IOException> gets thown, should I still do a <prntWrtr.close()>, also
    >probably in the <catch> block?


    Same answer. There is no PrintWriter instance for you to close.

    >Or in each such situation can I
    >conclude that since an exception occurred while I was attempting to
    >open the respective file, the variables will still each both be
    ><null>, and therefore I don't have to do anything?


    Correct. However, a little bit of care must be taken if you have multiple
    statements in the try/catch block. If the creation of the object succeeds,
    but a later usage throws an exception, you should close it.

    It's not uncommon to see code like:
    finally {
    if (someResource != null) {
    try {
    someResource.close();
    } catch (exceptionsCloseCanThrow ecct) {
    logger.error("WTF!", ecct);
    }
    }
    }

    This is ugly, and an admission that you haven't tracked the state of your
    resource very well, but it does always make sure you've closed the resource if
    possible.
    --
    Mark Rafn <http://www.dagon.net/>
     
    Dagon, Mar 15, 2011
    #5
  6. KevinSimonson

    Lew Guest

    Dagon wrote:
    > It's not uncommon to see code like:
    > finally {
    > if (someResource != null) {
    > try {
    > someResource.close();
    > } catch (exceptionsCloseCanThrow ecct) {
    > logger.error("WTF!", ecct);
    > }
    > }
    > }
    >
    > This is ugly, and an admission that you haven't tracked the state of your
    > resource very well, but it does always make sure you've closed the resource if
    > possible.


    Until the new resource-release idioms come on line, there's a certain ugliness
    that's unavoidable.

    I like the assign-once idioms, i.e., no initial assignment to 'null', and
    therefore assertable non-nullity.

    Something like:

    public void useResource( String src, String dst )
    {
    final BufferedReader reader;
    try
    {
    reader = new BufferedReader( new FileReader( src ));
    }
    catch ( IOException exc )
    {
    final String msg = "cannot open \"+ src +"\". "
    + exc.getLocalizedMessage();
    logger.error( msg, exc );
    throw new IllegalStateException( msg, exc );
    }
    assert reader != null;

    final BufferedWriter writer;
    try
    {
    writer = new BufferedWriter( new FileWriter( dst ));
    }
    catch ( IOException exc )
    {
    final String msg = "cannot open \"+ dst +"\". "
    + exc.getLocalizedMessage();
    logger.error( msg, exc );
    try
    {
    reader.close();
    }
    catch ( IOException cex )
    {
    final String cmsg = "cannot close \"+ src +"\". "
    + cex.getLocalizedMessage();
    logger.error( cmsg, cex );
    }
    throw new IllegalStateException( msg, exc );
    }
    assert writer != null;

    try
    {
    workWith( reader, writer ); // no closing inside that method
    }
    finally
    {
    try
    {
    reader.close();
    }
    catch ( IOException cex )
    {
    final String cmsg = "cannot close \"+ src +"\". "
    + cex.getLocalizedMessage();
    logger.error( cmsg, cex );
    }
    try
    {
    writer.close();
    }
    catch ( IOException cex )
    {
    final String cmsg = "cannot close \"+ dst +"\". "
    + cex.getLocalizedMessage();
    logger.error( cmsg, cex );
    }
    }
    }

    --
    Lew
    Honi soit qui mal y pense.
     
    Lew, Mar 15, 2011
    #6
  7. In message <>, Dagon wrote:

    > It's not uncommon to see code like:
    > finally {
    > if (someResource != null) {
    > try {
    > someResource.close();
    > } catch (exceptionsCloseCanThrow ecct) {
    > logger.error("WTF!", ecct);
    > }
    > }
    > }
    >
    > This is ugly, and an admission that you haven't tracked the state of your
    > resource very well ...


    What’s the alternative?
     
    Lawrence D'Oliveiro, Mar 15, 2011
    #7
  8. On 15 Mrz., 08:52, Lawrence D'Oliveiro <l...@geek-
    central.gen.new_zealand> wrote:
    > In message <>, Dagon wrote:
    >
    > > It's not uncommon to see code like:
    > >     finally {
    > >         if (someResource != null) {
    > >             try {
    > >                 someResource.close();
    > >             } catch (exceptionsCloseCanThrow ecct) {
    > >                 logger.error("WTF!", ecct);
    > >             }
    > >         }
    > >     }

    >
    > > This is ugly, and an admission that you haven't tracked the state of your
    > > resource very well ...

    >
    > What’s the alternative?


    The alternative is to create and assign outside the block and do the
    close in finally - unconditionally:

    final FileInputStream fin = new FileInputStream("foo.txt");
    try {
    final byte[] buffer = new byte[1024];
    for (int read;(read = fin.read(buffer))>=0;) {
    // do whatever with buffer[0,read[
    }
    }
    finally {
    // no null or other check needed!
    fin.close();
    }

    If you want to ensure an exception from close() does not shadow
    another error you can do

    static void close(Closable c) {
    try {
    c.close();
    }
    catch (IOException e) {
    // ignore or log
    }
    }

    and use that in the finally block.

    If you want BufferedReader you can do

    final BufferedReader reader =
    new BufferedReader(new InputStreamReader(new
    FileInputStream("foo.txt")));
    try {
    for (String line; (line = reader.readLine()) != null;) {
    System.out.println("Found line: " + line);
    }
    }
    finally {
    reader.close();
    }

    I would declare IOException on the method that contains this code to
    leave IO error handling to callers.

    Kind regards

    robert
     
    Robert Klemme, Mar 15, 2011
    #8
  9. On Tue, 15 Mar 2011 03:41:15 -0700, Robert Klemme wrote:

    >
    > If you want BufferedReader you can do
    >
    > final BufferedReader reader =
    > new BufferedReader(new InputStreamReader(new
    > FileInputStream("foo.txt")));
    > try {
    > for (String line; (line = reader.readLine()) != null;) {
    > System.out.println("Found line: " + line);
    > }
    > }
    > finally {
    > reader.close();
    > }


    That only works because both InputStreamReader and BufferedReader
    constructors don't throw.
    But it is not safe to do the same for:
    final ObjectInputStream ois =
    new ObjectInputStream(new FileInputStream("foo.txt"));

    Sometimes it would be good to have destructors...

    --
    Michal
     
    Michal Kleczek, Mar 15, 2011
    #9
  10. KevinSimonson

    Lew Guest

    On 03/15/2011 06:53 AM, Michal Kleczek wrote:
    > On Tue, 15 Mar 2011 03:41:15 -0700, Robert Klemme wrote:
    >
    >>
    >> If you want BufferedReader you can do
    >>
    >> final BufferedReader reader =
    >> new BufferedReader(new InputStreamReader(new
    >> FileInputStream("foo.txt")));
    >> try {
    >> for (String line; (line = reader.readLine()) != null;) {
    >> System.out.println("Found line: " + line);
    >> }
    >> }
    >> finally {
    >> reader.close();
    >> }

    >
    > That only works because both InputStreamReader and BufferedReader
    > constructors don't throw.
    > But it is not safe to do the same for:
    > final ObjectInputStream ois =
    > new ObjectInputStream(new FileInputStream("foo.txt"));


    That's covered upthread five and a half hours prior to the cited post.

    > Sometimes it would be good to have destructors...


    Nonsense. Destructors are to release memory. This is about external
    resources. What Java needs are 'finally' blocks. Hey, good news! It's got
    them! Yay! Problem solved!

    --
    Lew
    Honi soit qui mal y pense.
     
    Lew, Mar 15, 2011
    #10
  11. On Tue, 15 Mar 2011 07:54:17 -0400, Lew wrote:

    > On 03/15/2011 06:53 AM, Michal Kleczek wrote:
    >> On Tue, 15 Mar 2011 03:41:15 -0700, Robert Klemme wrote:
    >>
    >>
    >>> If you want BufferedReader you can do
    >>>
    >>> final BufferedReader reader =
    >>> new BufferedReader(new InputStreamReader(new
    >>> FileInputStream("foo.txt")));
    >>> try {
    >>> for (String line; (line = reader.readLine()) != null;) {
    >>> System.out.println("Found line: " + line);
    >>> }
    >>> }
    >>> finally {
    >>> reader.close();
    >>> }

    >>
    >> That only works because both InputStreamReader and BufferedReader
    >> constructors don't throw.
    >> But it is not safe to do the same for: final ObjectInputStream ois =
    >> new ObjectInputStream(new FileInputStream("foo.txt"));

    >
    > That's covered upthread five and a half hours prior to the cited post.
    >


    Maybe I'm missing it but could not find anything related.
    If you're talking about your post - there is still this
    new BufferedReader(new ...) thingy hanging around there
    and nothing about watching out for leaked resources if both
    chained constructors can throw.

    >> Sometimes it would be good to have destructors...

    >
    > Nonsense. Destructors are to release memory.


    Who told you that? :)

    > This is about external
    > resources.


    And that is also something that destructors can (and should) release.
    Please - google "RAII".

    > What Java needs are 'finally' blocks. Hey, good news! It's
    > got them! Yay! Problem solved!


    The point is they are not really straightforward to use
    and cause a lot of clutter in the code.
    This is not only my opinion: introduction of "with" statement in C#,
    proposed changes to Java 7 and this thread itself speak for themselves.

    --
    Michal
     
    Michal Kleczek, Mar 15, 2011
    #11
  12. On 15 Mrz., 11:53, Michal Kleczek <> wrote:
    > On Tue, 15 Mar 2011 03:41:15 -0700, Robert Klemme wrote:
    >
    > > If you want BufferedReader you can do

    >
    > > final BufferedReader reader =
    > >   new BufferedReader(new InputStreamReader(new
    > > FileInputStream("foo.txt")));
    > > try {
    > >   for (String line; (line = reader.readLine()) != null;) {
    > >     System.out.println("Found line: " + line);
    > >   }
    > > }
    > > finally {
    > >   reader.close();
    > > }

    >
    > That only works because both InputStreamReader and BufferedReader
    > constructors don't throw.
    > But it is not safe to do the same for:
    > final ObjectInputStream ois =
    >   new ObjectInputStream(new FileInputStream("foo.txt"));


    You just need to apply the same pattern several times:

    final new FileInputStream fin = FileInputStream("foo.txt");
    try {
    final ObjectInputStream ois = new ObjectInputStream(fin);
    try {
    ...
    }
    finally {
    ois.close();
    }
    }
    finally {
    // multiple close() do not hurt!
    fin.close();
    }

    Or you create a separate method for opening

    private static ObjectInputStream open(String fileName) throws
    IOException {
    final new FileInputStream fin = FileInputStream("foo.txt");
    boolean ok = false;
    try {
    final ObjectInputStream oin = new ObjectInputStream(fin);
    ok = true;
    return oid;
    }
    finally {
    if (!ok) fin.close();
    }
    }

    Or, a bit shorter:

    private static ObjectInputStream open(String fileName) throws
    IOException {
    final new FileInputStream fin = FileInputStream("foo.txt");
    try {
    return new ObjectInputStream(fin);
    }
    catch (IOException e) {
    fin.close();
    throw e;
    }
    }

    Kind regards

    robert
     
    Robert Klemme, Mar 15, 2011
    #12
  13. KevinSimonson

    Lew Guest

    Michal Kleczek wrote:
    > Lew wrote:
    > > Michal Kleczek wrote:
    > >> Robert Klemme wrote:

    >
    > >>> If you want BufferedReader you can do

    >
    > >>> final BufferedReader reader =
    > >>>    new BufferedReader(new InputStreamReader(new
    > >>> FileInputStream("foo.txt")));
    > >>> try {
    > >>>    for (String line; (line = reader.readLine()) != null;) {
    > >>>      System.out.println("Found line: " + line);
    > >>>    }
    > >>> }
    > >>> finally {
    > >>>    reader.close();
    > >>> }

    >
    > >> That only works because both InputStreamReader and BufferedReader
    > >> constructors don't throw.
    > >> But it is not safe to do the same for: final ObjectInputStream ois =
    > >>    new ObjectInputStream(new FileInputStream("foo.txt"));

    >
    > > That's covered upthread five and a half hours prior to the cited post.

    >
    > Maybe I'm missing it but could not find anything related.
    >


    It's my post from Mar 15, 1:17 am, New York time.

    > If you're talking about your post - there is still this
    >


    Well, duhhh.

    > new BufferedReader(new ...) thingy hanging around there
    > and nothing about watching out for leaked resources if both
    > chained constructors can throw.


    Huh? Yes there is, too - it's called a 'catch' block. Notice that if
    the constructors throw an exception in my example that it's caught and
    all resources are guaranteed to be released, through the use of
    'finally'.

    You *did* notice that "this new BufferedReader(new ...) thingy hanging
    around there" was inside a 'try...catch' structure, didn't you? How
    is that "nothing about watching out for leaked resources"? You *did*
    notice that the 'catch' blocks release resources if the whole shebang
    doesn't allocate, didn't you? You *did* notice the 'finally' block
    that guarantees (RAII-ishly!) release of resources at the end, didn't
    you? If not, how could you possibly miss all that code in my post?
    It was the largest part of it!

    The whole freaking *point* of my post was to show one way to guarantee
    release of resources!

    Look, make all the legitimate points you want, but don't lie.

    > >> Sometimes it would be good to have destructors...

    >
    > > Nonsense.  Destructors are to release memory.  

    >
    > Who told you that? :)
    >
    > > This is about external
    > > resources.  

    >
    > And that is also something that destructors can (and should) release.
    >


    It's something that should be released, but not necessarily in a
    destructor.

    For proof, consider that Java doesn't have destructors yet you are
    still perfectly capable of releasing resources in a guaranteed way
    using 'finally'.

    > Please - google "RAII".
    >


    Which is what Java uses 'finally' for.

    > > What Java needs are 'finally' blocks.  Hey, good news!  It's
    > > got them!  Yay!  Problem solved!

    >
    > The point is they are not really straightforward to use
    > and cause a lot of clutter in the code.
    > This is not only my opinion: introduction of "with" statement in C#,
    > proposed changes to Java 7 and this thread itself speak for themselves.
    >


    Yes, those add a little sugar, but they don't add new capability.

    I guess one man's "clutter" is another man's explicit logic.

    See my post that you erroneously claimed had "nothing" to release
    resources for an example of how to guarantee resource release in Java
    using 'try...catch...finally'.

    --
    Lew
     
    Lew, Mar 15, 2011
    #13
  14. On Tue, 15 Mar 2011 10:56:46 -0700, Lew wrote:

    > Michal Kleczek wrote:
    >> Lew wrote:
    >> > Michal Kleczek wrote:
    >> >> Robert Klemme wrote:

    >>
    >> >>> If you want BufferedReader you can do

    >>
    >> >>> final BufferedReader reader =
    >> >>>    new BufferedReader(new InputStreamReader(new
    >> >>> FileInputStream("foo.txt")));
    >> >>> try {
    >> >>>    for (String line; (line = reader.readLine()) != null;) {
    >> >>>      System.out.println("Found line: " + line);
    >> >>>    }
    >> >>> }
    >> >>> finally {
    >> >>>    reader.close();
    >> >>> }

    >>
    >> >> That only works because both InputStreamReader and BufferedReader
    >> >> constructors don't throw.
    >> >> But it is not safe to do the same for: final ObjectInputStream ois =
    >> >>    new ObjectInputStream(new FileInputStream("foo.txt"));

    >>


    [snip]
    > The whole freaking *point* of my post was to show one way to guarantee
    > release of resources!
    >
    > Look, make all the legitimate points you want, but don't lie.


    You're right - I've read your post again and I admit your way of handling
    this does not have problems I wanted to raise. But I was not replying to
    _your_ post but to the one cited at the top.
    If you just wanted to point out that your way of managing resources
    described in another branch of this thread is better - then point taken.

    >
    >> >> Sometimes it would be good to have destructors...

    >>
    >> > Nonsense.  Destructors are to release memory.

    >>
    >> Who told you that? :)
    >>
    >> > This is about external
    >> > resources.

    >>
    >> And that is also something that destructors can (and should) release.
    >>
    >>

    > It's something that should be released, but not necessarily in a
    > destructor.


    True. But:
    1. Surely destructors are not only to release memory.
    2. _Most of the times_ resources acquired during the lifetime of an
    object should be released in the destructor.

    >
    > For proof, consider that Java doesn't have destructors yet you are still
    > perfectly capable of releasing resources in a guaranteed way using
    > 'finally'.


    I didn't say anywhere that programs written in Java are not capable of
    releasing resources in a guaranteed way.
    What I said was:

    1. This idiom:

    final InputStreamWrapper wrapper =
    new InputStreamWrapper(new ResourceAcquiringInputStream(resource));
    try {
    //process wrapper
    }
    finally {
    wrapper.close();
    }

    while common is dangerous because it _may_ lead to resource leaks.

    2. In this particular case (resource acquiring/releasing) I miss
    destructors in Java since try/catch/finally construct makes code less
    readable when I need to manage several resources.

    --
    Michal
     
    Michal Kleczek, Mar 15, 2011
    #14
  15. In message <ilnget$v35$>, Michal Kleczek wrote:

    > Sometimes it would be good to have destructors...


    You want a close/finalize call that will never return an error.
     
    Lawrence D'Oliveiro, Mar 15, 2011
    #15
  16. In message <iloifu$v36$>, Michal Kleczek wrote:

    > 2. In this particular case (resource acquiring/releasing) I miss
    > destructors in Java since try/catch/finally construct makes code less
    > readable when I need to manage several resources.


    I wrote about a scalable technique for dealing with this years ago
    <http://www.geek-central.gen.nz/peeves/programming_discipline.html>.
     
    Lawrence D'Oliveiro, Mar 15, 2011
    #16
  17. KevinSimonson

    Roedy Green Guest

    The key to understanding is this. If your open fails there will be no
    Stream object created, and there will be no reference to it in your
    handle. If you try to close it, you will get a NullPointerException.
    If the logic lets you get to your close without a successful open do
    this:

    if ( handle != null ){ handle.close();}
    --
    Roedy Green Canadian Mind Products
    http://mindprod.com
    Refactor early. If you procrastinate, you will have
    even more code to adjust based on the faulty design.
    ..
     
    Roedy Green, Mar 16, 2011
    #17
  18. Michal Kleczek wrote:

    > On Tue, 15 Mar 2011 10:56:46 -0700, Lew wrote:
    >
    >> Michal Kleczek wrote:
    >>> Lew wrote:
    >>> > Michal Kleczek wrote:
    >>> >> Robert Klemme wrote:
    >>>
    >>> >>> If you want BufferedReader you can do
    >>>
    >>> >>> final BufferedReader reader =
    >>> >>> new BufferedReader(new InputStreamReader(new
    >>> >>> FileInputStream("foo.txt")));
    >>> >>> try {
    >>> >>> for (String line; (line = reader.readLine()) != null;) {
    >>> >>> System.out.println("Found line: " + line);
    >>> >>> }
    >>> >>> }
    >>> >>> finally {
    >>> >>> reader.close();
    >>> >>> }
    >>>
    >>> >> That only works because both InputStreamReader and BufferedReader
    >>> >> constructors don't throw.
    >>> >> But it is not safe to do the same for: final ObjectInputStream ois =
    >>> >> new ObjectInputStream(new FileInputStream("foo.txt"));
    >>>

    >
    > [snip]
    >> The whole freaking *point* of my post was to show one way to guarantee
    >> release of resources!
    >>
    >> Look, make all the legitimate points you want, but don't lie.

    >
    > You're right - I've read your post again and I admit your way of handling
    > this does not have problems I wanted to raise.


    I've re-read your post again and I think my point holds. This code (copied
    from your post):

    > public void useResource( String src, String dst )
    > {
    > final BufferedReader reader;
    > try
    > {
    > reader = new BufferedReader( new FileReader( src ));
    > }
    > catch ( IOException exc )
    > {
    > final String msg = "cannot open \"+ src +"\". "
    > + exc.getLocalizedMessage();
    > logger.error( msg, exc );
    > throw new IllegalStateException( msg, exc );
    > }
    > assert reader != null;


    is not exception safe - if BufferedReader constructor throws you've got a
    resource leak.

    [snip]
    >>> > Sometimes it would be good to have destructors...
    >>>.>
    >> > Nonsense. Destructors are to release memory.
    >> > What Java needs are 'finally' blocks. Hey, good news! It's
    >> > got them! Yay! Problem solved!


    As you can see above the problem is not solved. Above code would be
    perfectly valid (in terms of exception safety) if we had destructors (and -
    of course - if FileReader destructor was implemented to release resources
    propely).

    --
    Michal
     
    Michal Kleczek, Mar 16, 2011
    #18
  19. KevinSimonson

    Lew Guest

    Michal Kleczek wrote:
    > You're right - I've read your post again and I admit your way of handling
    > this does not have problems I wanted to raise. But I was not replying to
    > _your_ post but to the one cited at the top.
    > If you just wanted to point out that your way of managing resources
    > described in another branch of this thread is better - then point taken.


    Not better, just possible.

    --
    Lew
    Honi soit qui mal y pense.
     
    Lew, Mar 16, 2011
    #19
  20. KevinSimonson

    Lew Guest

    Michal Kleczek wrote:
    >> public void useResource( String src, String dst )
    >> {
    >> final BufferedReader reader;
    >> try
    >> {
    >> reader = new BufferedReader( new FileReader( src ));
    >> }
    >> catch ( IOException exc )
    >> {
    >> final String msg = "cannot open \"+ src +"\". "
    >> + exc.getLocalizedMessage();
    >> logger.error( msg, exc );
    >> throw new IllegalStateException( msg, exc );
    >> }
    >> assert reader != null;

    >
    > is not exception safe - if BufferedReader constructor throws you've got a
    > resource leak.


    Hence the 'catch' block. What resource can leak there?

    --
    Lew
    Honi soit qui mal y pense.
     
    Lew, Mar 16, 2011
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?Q2hyeXNhbg==?=

    Why popup window will be re-opened again after closing it?

    =?Utf-8?B?Q2hyeXNhbg==?=, Sep 29, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    2,324
    =?Utf-8?B?Q2hyeXNhbg==?=
    Oct 3, 2005
  2. Maxim Khesin

    closing file opened by csv reader

    Maxim Khesin, Dec 9, 2003, in forum: Python
    Replies:
    1
    Views:
    310
    Paul Phillabaum
    Dec 9, 2003
  3. Francis Hwang

    Closing files opened with Open3.popen

    Francis Hwang, Jun 2, 2004, in forum: Ruby
    Replies:
    2
    Views:
    174
    Paul Brannan
    Jun 2, 2004
  4. Arun Seetharam
    Replies:
    8
    Views:
    134
    Grant Wagner
    Jul 27, 2004
  5. Replies:
    10
    Views:
    190
    Anno Siegel
    May 6, 2006
Loading...

Share This Page