Search code examples
javainputstreamoutputstream

Close In/OutputStream in Java


As u can see, i did close the the fo and oo, so after that do i need to close the OutputStream ? :

public void writedata(List<sinhvien> svlist) {
    FileOutputStream fo = null;
    ObjectOutputStream oo = null;
    try {
        fo = new FileOutputStream(datasv);
        oo = new ObjectOutputStream (fo);
        oo.writeObject(svlist);
    }
    catch (IOException e) {}
    
    finally {
            try {
                fo.close();
                oo.close();
            }
            catch (IOException e ) {}
    }
}

Somethings like : ... OutputStream os if (os != null ) try { os.close() } ... Is this necessary ?


Solution

  • You're doing it in the wrong order.

    streams are allowed to buffer. By closing the underlying stream (fo) first, if oo buffered, your code fails. You are also eating any exceptions which is obviously an extremely bad idea: If your code fails (for example, that oo close throws an exception, reporting that it failed to write out the last few bytes because fo is already closed, you'd never know, and now you have a corrupt file and no idea that it is corrupted.

    You could just close oo and be done with it, but the problem is, what if oo = new ObjectOutputStream(fo) fails? You'd still need to close fo then.

    The lessons:

    1. You have to close each resource that you open, in stack order (the last thing you made needs to be closed first).
    2. Do not, ever, write an empty catch block unless you really, really, really know what you are doing.

    The best way to apply these lessons:

    1. Use try-with-resources
    2. Freely litter throws IOException and the like on your signatures. your main should be declared as throws Exception. Where not feasible or possible, the right 'I do not know what happened' catch block is: `catch (Thingie e) { throw new RuntimeException("uncaught", e);

    Thus:

    public void writedata(List<sinhvien> svlist) throws IOException {
        try (var fo = new FileOutputStream(datasv);
             var oo = new ObjectOutputStream(fo)) {
    
            oo.writeObject(svlist);
        }
    }
    

    is orders of magnitude better:

    1. It doesn't suffer from caching issues
    2. It is much easier to read and follow
    3. a method called 'writeData' can obviously exit in the alternate way of: Hmm, whoops, issues with the I/O system that I'm saving it to. methods should throw what seems obvious, and IOException is obvious here.
    4. does not silently do nothing when saving the data fails.