java - copying files in android with input/output stream: the good and the bad way

Question

i have made this two routines to copy files using inputstream and outpustream. they are quite the same however the second one rise ArrayIndexOutOfBoundsException while the first one works flawlessly and i don't know why:

    public void CopyStream(long size, InputStream is, OutputStream os) {
        final int buffer_size = 4096;
        byte[] bytes = new byte[buffer_size];
        try {
            int count,prog=0;
            while ((count = is.read(bytes)) != -1) {
                os.write(bytes, 0, count); //write buffer
                prog = prog + count;
                publishProgress(((long) prog) * 100 / size);
            }
            os.flush();
            is.close();
            os.close();
        } catch (Exception ex) {
            Log.e(TAG,"CS "+ex);
        }
    }

as you may guess the routine is called inside an AsyncTask, therefore the publishProgresss

    public void CopyStream(long size, InputStream is, OutputStream os) {
        final int buffer_size = 4096;
        try {
            byte[] bytes = new byte[buffer_size];
            for (int count=0,prog=0;count!=-1;) {
                count = is.read(bytes);
                os.write(bytes, 0, count);
                prog=prog+count;
                publishProgress(((long) prog)*100/size);
            }
            os.flush();
            is.close();
            os.close();
        } catch (Exception ex) {
            Log.e(TAG,"CS "+ex);
        }
    }

Does anyone know why the while works but the for no ? what am i missing?

Answer 1

The problem lies in your for loop checking the condition after the first run through. Basically the error occurs when it has read fine the last loop but on the next loop the is.read call returns -1. Afterwards you try to call os.write(bytes,0,-1); -1 is an invalid index. The solution would be:

public void CopyStream(long size, InputStream is, OutputStream os) {
        final int buffer_size = 4096;
        try {
            byte[] bytes = new byte[buffer_size];
            for (int count=0,prog=0;count!=-1;) {
                count = is.read(bytes);
                if(count != -1) {
                  os.write(bytes, 0, count);
                  prog=prog+count;
                  publishProgress(((long) prog)*100/size);
                }
            }
            os.flush();
            is.close();
            os.close();
        } catch (Exception ex) {
            Log.e(TAG,"CS "+ex);
        }
    }

But it is much more readable as the while loop so I would stick with that. For loops should be used either when you know the quantity of times to loop or as a for each where you loop through each individual item of a collection.