Copy file using read and write streams

Question

I have this piece of code in my program, where I assign 10000 as the size of the buffer. Now How can I check if the file that I am copying is less than 10000, for example 5000, than how to free the remaining 5000 back?

Thank You.

private void copyFile(File in, File out) {
        var buffer = new byte[10000];
        try {
            FileInputStream dis = new FileInputStream(in);
            FileOutputStream dos = new FileOutputStream(out);
            int count;
            do {
                count = dis.read(buffer);
                if (count != -1)
                    dos.write(buffer, 0, count);
            } while (count != -1);
            dis.close();
            dos.close();
        } catch (IOException e) {
            program.print("Error copying file: " + e.toString() + "\n", null);
        }
    }

EDIT: Is there anyway, I can optimize this more?

You don't free memory. The garbage collector, which runs occasionally, frees memory. Your array will be collected at some point after it goes out of scope. There is a way to ask the GC politely to run. Don't. It knows what it's doing. — Eric Stein
– Eric Stein, Commented Nov 13, 2020 at 4:32
PLEASE learn your APIs. Copying files by hand is not even yesterday anymore. See java.nio.file.Files.copy() — mtj
– mtj, Commented Nov 13, 2020 at 6:27

Eric Stein · Accepted Answer · 2020-11-13 04:55:11Z

Streams should always be closed. You will have resource leaks if your method throws an exception. Either close the streams in a finally block, or use try-with-resources to make sure they get closed.

Optional curly braces aren't. Skipping them will often lead to bugs when code gets modified later.

The typical loop structure used for working with streams looks more like

int charsRead;
while ((charsRead = dis.read(buffer) > 0) {
    // do stuff
}

10,000 is pretty big for a buffer. That may not be the best choice.

Please don't swallow the stack trace. That's critical information when this code needs to be debugged later.

Opinion: var is icky.

If you apply all these changes, your code might look more like:

private void copyFile(File in, File out) {
    byte[] buffer = new byte[1024];
    try (FileInputStream dis = new FileInputStream(in);
            FileOutputStream dos = new FileOutputStream(out)) {
        int charsRead;
        while ((buffer = dis.read(buffer) > 0) {
                dos.write(buffer, 0, charsRead);
        }
    } catch (IOException e) {
        //I don't know how to use your logging library to display exception stack trace, so..
        e.printStackTrace(System.err);
    }
}

Of course, this is assuming that you can't use the rather intuitively named Files.copy() instead. That would be much better.

private void copyFile(File in, File out) {
    try {
        Files.copy(in.toPath(), out.toPath());
    } catch (IOException e) {
        e.printStackTrace(System.err);
    }
}

Ralf Kleberhoff · Accepted Answer · 2020-11-13 14:45:03Z

A remark on exception handling.

Exceptions are meant to communicate to a method's caller that the method did not complete its job.

In your case, the job is to copy a file. And if, during that process, an exception happens, then most probably, the out file hasn't become a valid copy of the in file. You have to inform your caller of that failure. Informing the user instead is wrong for two reasons:

If not receiving an exception, your caller method assumes everything was done, will maybe later read that file copy, and get an error or incomplete data, being hard to debug.
As your method's job has nothing to do with user interface, it's strange to talk to the user. Some top-level activity has to receive exceptions and should know how to inform the user.

So, it's better to just declare the exceptions that can happen, and let some instance up the call stack decide how to deal with that. Your method should just report its failure to its caller and let him deal with it:

private void copyFile(File in, File out) throws IOException {
    var buffer = new byte[10000];
    
    FileInputStream dis = new FileInputStream(in);
    FileOutputStream dos = new FileOutputStream(out);
    int count;
    do {
        count = dis.read(buffer);
        if (count != -1)
            dos.write(buffer, 0, count);
    } while (count != -1);
    dis.close();
    dos.close();
}

Dee Jai · Accepted Answer · 2020-12-02 14:08:39Z

0

Few things to be looked into

There should be a finally block where all the stream open should be closed, this will make sure that there will not be any open stream even in case of exception from loop.

https://www.javatpoint.com/finally-block-in-exception-handling

Function used for printing exception needs to be changed to System.err or e.printstacktrace().

answered Dec 2, 2020 at 14:08

Dee Jai

213 bronze badges

Add a comment |

Stack Exchange Network

Copy file using read and write streams

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Copy file using read and write streams

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions