how to close a stream in a try/catch block

Discussion in 'Java' started by jtl.zheng, Aug 8, 2006.

  1. jtl.zheng

    jtl.zheng Guest

    I have written a method to compare two file:

    -----------------------------------------------------------------
    public static boolean compareFile(File file1, File file2) {
    BufferedInputStream in1, in2;
    try {
    in1 = new BufferedInputStream(new FileInputStream(
    file1));
    in2 = new BufferedInputStream(new FileInputStream(
    file2));
    int i;
    while ( (i = in1.read()) != -1) {
    if (i != in2.read()) {
    in1.close();
    in2.close();
    return false;
    }
    }
    if (in2.read() != -1) {
    in1.close();
    in2.close();
    return false;
    }
    in1.close();
    in2.close();
    return true;
    }
    catch (FileNotFoundException ex) {
    ex.printStackTrace();
    }
    catch (IOException ex) {
    ex.printStackTrace();
    }
    finally {
    //in1.close(); // compile error:might not been initialized
    //in2.close();
    }
    //in1.close(); // compile error:might not been initialized
    //in2.close();
    return false;
    }
    ----------------------------------------------------------------

    I can't put the in1.close() into the finally block
    the compiler say" variable in1 might not have been initialized"

    so I must write it three times before every return sentence
    it's so bothering...do you have any better idea?

    and it still have a problem
    when it catch a exception,it will not reach the in1.close()
    so when it bring on exception the stream can't be closed

    Thank you very much in advance
    : )
     
    jtl.zheng, Aug 8, 2006
    #1
    1. Advertisements

  2. jtl.zheng

    jmcgill Guest

    Here's a rough idea:

    Declare in1 and in2 in the scope where you have them, but initialize
    them to null. Also, initialize your return value in this same outer scope.

    Then, in one try block, open the Streams on them. Catch the File and IO
    errors that can happen here.

    In another try block, do the operations in your loop. Set the return
    value as appropriate.

    Then, in a finally block, close the Streams. This is the *only* place
    you close them. Then return the boolean.

    That's the basic idea, I think. Too tired to actually try to write it.
     
    jmcgill, Aug 8, 2006
    #2
    1. Advertisements

  3. Hi,

    Sure, you didn't initialize them :)
    Close them in the finally block.

    public static boolean compareFile(File file1, File file2) {
    BufferedInputStream in1 = null;
    BufferedInputStream in2 = null;

    try {
    in1 = new BufferedInputStream(new FileInputStream(
    file1));
    in2 = new BufferedInputStream(new FileInputStream(
    file2));

    boolean result = true;
    int i;
    do {
    i = in1.read();
    result = (i == in2.read());
    } while ( result && i != -1 );

    return result;
    }
    catch (FileNotFoundException ex) {
    ex.printStackTrace();
    }
    catch (IOException ex) {
    ex.printStackTrace();
    }
    finally {
    close(in1);
    close(in2);
    }

    return false;
    }

    private static void close( InputStream is ) {
    if ( is != null ) {
    try {
    is.close();
    }
    catch ( IOException ioe ) {
    ioe.printStackTrace();
    }
    }
    }

    Bye
    Michael
     
    Michael Rauscher, Aug 8, 2006
    #3
  4. jtl.zheng

    jtl.zheng Guest

    in jmcgill's:
    it is a good idea !
    Thank you very much!


    in Michael Rauscher's
    A good idea too !
    Thank you very much!
     
    jtl.zheng, Aug 8, 2006
    #4
  5. In fact, I don't think that it's a good idea to swallow exceptions.

    If the method compareFile returns false nobody knows if there are
    differences between the two files or if the method returned false due to
    an error (exception).

    So, IMO it would be much clearer to let the methods throw an IOException.

    Bye
    Michael
     
    Michael Rauscher, Aug 8, 2006
    #5
  6. jtl.zheng

    jtl.zheng Guest

    Thank you for your advice.

    I also have puzzle in handling IOException

    in my mind I would like to catch all the IOException but not to throws
    them.because throw IOException will make the program terninate.
    but when catch IOException I am fear to make someting wrong because I
    may not deal with the IOException correctly.

    how do you usually to do to deal with IOException?
    do you catch all the Exception?
    and how would you deal with the Excepton? and then call the method one
    more again?

    you may not understand what I am talking about...
    I am new in Java and have very little experience how to deal with these
    problems..

    but still Thank you all the same
    : )
     
    jtl.zheng, Aug 8, 2006
    #6
  7. If you get an exception while closing an InputStream there's really
    nothing you can do except perhaps log it. No data will have been lost
    and there is no harm done. I usually ignore this kind of exception.

    If you get an exception while closing an OutputStream, there is a risk
    that data you wrote has not been written to the underlying device.
    Depending on the situation you may need to try again, or notify the
    user that the operation failed.

    It really depends on the situation.

    /gordon
     
    Gordon Beaton, Aug 8, 2006
    #7
  8. jtl.zheng

    jtl.zheng Guest

    and there are so much unchecked Exceptions in a program
    like NullPointerException, IndexOutOfBoundsExceptions
    do you catch all these Exception inside the program?
    how do you deal with them usually ?

    I am still a student and maybe these question are so stupidity.
    : )
     
    jtl.zheng, Aug 8, 2006
    #8
  9. jtl.zheng

    Chris Uppal Guest

    The important distinction is between exceptions which only indicate a bug in
    the program, and those which indicate a problem which you (the programmer)
    can't control in advance.

    The distinction isn't completely black-and-white, but that's still the way to
    start thinking about exceptions and what to do with them.

    For many programs, there is no need to handle problems caused by bugs -- it's
    better just to let the program die with a stack trace. These are the problems
    that are not supposed to happen after you've finished testing your program (but
    of course they still do ;-). But whether they happen during testing or
    afterwards, it is usually best just to let them happen, find out what caused
    them, and fix the code. NullPointerException and IndexOutOfBoundsExceptions
    are examples of this kind of exception.

    There are some cases where your program has to protect itself even against its
    own bugs. Two examples might be a program which has pluggable components (such
    as a servlet container) which has to try to protect itself against bugs in the
    components. Another example would be an application such as a text editor,
    where it is important to make the best possible effort to save the user's work
    somehow, even if the program is in the middle of crashing.

    But the IO exceptions are not like that. The signal problems which the
    programmer knows may happen, but has no way of preventing by careful
    programming. For such errors, it is part of the application's functional
    design[*] how they are dealt with, so the only decision is where to put the
    logic for dealing with them. Often (but by no means always) it is a good idea
    to let exceptions propagate up to a fairly high level in the application, where
    a relatively simple and general handler can cope with all of them. (BTW,
    don't forget that you can re-throw exceptions, so you can catch them locally,
    do whatever local cleanup is needed, and then re-throw them to the higher-level
    hander which can take the appropriate action at a wider scale).

    In some very simple programs, such as examples posted to newsgroups and
    exercises for beginners, then best way to handle even anticipated problems like
    IO exceptions is just to let the program die. But not many real applications
    are like that -- imagine a text editor which crashed every time you tried to
    open a file that doesn't exist ;-)

    -- chris

    [*] By "functional design" I mean that the program's response to these problems
    is part of what the program is supposed to /do/ (how you would describe it to a
    customer, say) rather than part of the implementation design (how the code is
    structured).
     
    Chris Uppal, Aug 8, 2006
    #9
  10. jtl.zheng

    steve Guest

    well , really you should have a flush() before closing an output stream, it
    will at least go some way to ensuring that data is not lost.

    Steve
     
    steve, Aug 8, 2006
    #10
  11. jtl.zheng

    steve Guest

    it depends on your program, only a programmer knows how serious an error is.

    null & index out of bound , can usually be covered by correctly checking your
    input values.
    when you pass values to a routine, you should range check them inside the
    routine that is going to use them, because if you do not check them, then no
    one else is.

    yes i know it's a pain, and not as much fun as "real programming" but it has
    to be done.

    Steve
     
    steve, Aug 8, 2006
    #11
  12. jtl.zheng

    Chris Uppal Guest

    Why ?? OutputStream.close() is expected to do the equivalent of flushing[*]
    itself. Similarly for Writer().

    ([*] Although not required to call its own flush() method -- and in fact it
    doesn't)

    -- chris
     
    Chris Uppal, Aug 8, 2006
    #12
  13. jtl.zheng

    Chris Uppal Guest

    I feel this is rather misleading advice to give to a beginner. The point is
    that you, as a programmer, should know how much and what checking is required
    (given things like the source(s) of the inputs, the degree of trust you place
    in them, whether they form part of a co-designed and co-tested application,
    etc, etc). Testing and validating inputs just because you /can/ is not good
    programming -- in fact I would call it bad programming. But it's also true
    that not testing and validating inputs because you are too lazy is also bad
    programming.

    If you validate the input to some operation, then you should be able to
    describe (in terms of the application's architecture) /why/ you are validating
    it. Some good reasons include:
    It an input straight from the user.
    It is data from the database, and may be incorrect.
    It /should/ be OK, but it's easy to get wrong and the consequences
    are /dire/ if it is.
    Some bad reasons (i.e. non-reasons) include:
    Because that's the way I've been taught to do it.
    Because we sometimes get wrong values and are unwilling to spend the time
    to find out why (and fix it).
    Because it looks more "professional".
    Because it boosts my measured productivity in LOC/day ;-)

    Validating inputs amounts to duplication of logic (assuming that the app would
    fail if the incorrect input were allowed through -- which is normally the
    case). Duplication is more than just a waste of time, it is actively bad in
    that it
    introduces extra complexity
    introduces the possibility of false negatives (failing when it shouldn't)
    decreases flexibility.
    So (MO) validation is something you do only when you know you have a good
    reason for it which more than compensates for the downside.

    -- chris
     
    Chris Uppal, Aug 8, 2006
    #13
  14. Rather than doing null checking, I find it clearer to use try blocks to
    cover the exact required code. In the case of your code, the try should
    actually start in the middle of an expression...

    public static boolean compareFile(File file1, File file2) {
    try {
    FileInputStream fileIn1 = new FileInputStream(file1);
    try {
    FileInputStream fileIn2 = new FileInputStream(file2);
    try {
    InputStream in1 = new BufferedInputStream(fileIn1);
    InputStream in2 = new BufferedInputStream(fileIn2);
    ...
    return true;
    } finally {
    fileIn2.close();
    }
    } finally {
    fileIn1.close();
    }

    // Replace these with something more appropriate...

    } catch (FileNotFoundException ex) {
    ex.printStackTrace();
    } catch (IOException ex) {
    ex.printStackTrace();
    }
    return false;
    }

    In general it's a bad idea to write a method with code that covers so
    much - from File down to byte. There are ways of factoring out resource
    management code from many methods.

    Tom Hawtin
     
    Thomas Hawtin, Aug 9, 2006
    #14
  15. jtl.zheng

    jtl.zheng Guest

    Thanks very much to everyone here.
    I have learnt a lot about the exception and how to hendle it properly.
    : )
     
    jtl.zheng, Aug 11, 2006
    #15
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.