CODEWITHSUNDEEP
java
fhucho
fhucho

Reputation: 34560

How to download file and correctly handle exceptions?

My method for downloading file. It's a bit simplified, I removed the third argument - DownloadListener, which I use to inform the caller about dowonload progress.

public static boolean downloadFile(String url, File file) {
    try {
        HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
        connection.connect();

        FileOutputStream fos = new FileOutputStream(file);
        InputStream is = connection.getInputStream();

        byte[] buffer = new byte[1024];
        int len = 0;
        while ((len = is.read(buffer)) > 0) {
            fos.write(buffer, 0, len);
        }

        is.close();
        fos.flush();
        fos.close();
        return true;
    } catch (IOException e) {
        if (file.exists())
            file.delete();
        return false;
    }
}

I guess that the exceptions are not handled correctly but if I put the close() calls into finally block, they would have to be surrounded by try-catch block which would look very messy. There must be some cleaner way how to correctly download file in Java. Another thing is, should I call connection.disconnect()?

Upvotes: 0

Views: 1688

Answers (4)

Andrzej Doyle
Andrzej Doyle

Reputation: 103817

Not until Java 7's ARM. You're right, that often you need nested try-finally blocks within a finally block, in order to definitively clean up multiple resources. And that the correct way of doing it doesn't look very tidy if done in-line.

This is often a good candidate for extracting a static helper method (such as something like IOUtils.closeConnection()); the method can catch any exception, so as to avoid an exception stopping further resources from being closed.

Upvotes: 4

thotheolh
thotheolh

Reputation: 7450

Putting 'is' or 'fos' to close() in catch makes it unreachable.

Upvotes: 0

Buhake Sindi
Buhake Sindi

Reputation: 89189

My suggestion is that every resources that is utilized, must be closed (released) in the finally block. You don't want situations when you have open connection that isn't closed and trying to establish another connection and the the previous resource hasn't been released yet.

Upvotes: 2

Jigar Joshi
Jigar Joshi

Reputation: 240928

     FileOutputStream fos = null;
     InputStream is = null;
     try {
            HttpURLConnection connection = 
                   (HttpURLConnection) new URL(url).openConnection();
            connection.connect();

            fos = new FileOutputStream(file);

            is =  = connection.getInputStream();

            byte[] buffer = new byte[1024];
            int len = 0;
            while ((len = is.read(buffer)) > 0) {
                fos.write(buffer, 0, len);
            }


            return true;
        } catch (IOException e) {
            if (file.exists())
                file.delete();
            return false;
        }finally{
            myClose(is);
            myFlush(fos);
            myClose(fos);

   }     }

    public void myClose(Closable c){
      if(c == null)
         return;
      try{
         c.close();
      }catch(IOException ex)
        //perform necessary things
       }
    }
    public void myFlush(Flushable f){
      if(f == null)
        return;
      try{
         f.flush();
      }catch(IOException ex)
         //perform necessary things
   }

Upvotes: 1

Related Questions