Progress Bar isn't very accurate

Question

I'm working on my first android app, which is a cache cleaner for another very popular app. I've finally got functionality for checking the app cache, then deleting it if one exists. However, the Async task I've setup for deleting the files doesn't seem to update the progress bar accurately. As I am a programming n00b, my code is mostly copypasta from other sources.

Here's the block I suspect has the progress bar calculation issue:

    @Override
    protected Void doInBackground(Void... params) {

        // Delete Cache !!
        File dir = new File(Environment.getExternalStorageDirectory()
                + "/Android/Data/com.popularapp/Cache");

        // Progress Bar
        for (int i = 1; i < 100; i++) {
            if (isCancelled()) {
                break;
            } else {
                System.out.println(i);
                publishProgress(i);
                if (dir.isDirectory()) {
                    String[] children = dir.list();
                    for (int i2 = 0; i2 < children.length; i2++) {
                        new File(dir, children[i2]).delete();
                    }
                }
            }
        }

        return null;
    }

    @Override
    protected void onProgressUpdate(Integer... values) {
        super.onProgressUpdate(values);
        progressBar.setProgress(values[0]);
        tvLoading.setText("Loading...  " + values[0] + " %");
        tvPer.setText(values[0] + " %");
    }

When you're printing 'i' in your for loop to the console, is that number accurate each iteration? — Tonithy, Aug 14 '13 at 21:54
I had to figure out how to check the console for that kind of loop output, but upon further inspection it does seem to be counting each iteration properly. I did remove the outer loop as suggested, btw. — geek_sauce, Aug 14 '13 at 23:33

Tonithy · Accepted Answer · 2013-08-15T01:34:26.140

1

It seems as though your nested for loops are a unnecessary an you only need one to go through the directory. You should tie your progress to the iteration of the for loop that actually deletes the files.

You can remove your outer for loop and publish progress with your 'i2' counter variable.

Inside your second (now first) for loop call your publishProgress(i2 / children.length * 100). That should be a more accurate number, assuming all files take about the same amount of time to delete (probably a moderately safe assumption for your purposes).

@Override
protected Void doInBackground(Void... params) {

    // Delete Cache !!
    File dir = new File(Environment.getExternalStorageDirectory()
            + "/Android/Data/com.popularapp/Cache");
    if (dir.isDirectory()) {
        String[] children = dir.list();
         for (int i2 = 0; i2 < children.length; i2++) {
              if (isCancelled()) break;
              int progress = 100 * i2 / children.length;
              Log.w("Deleting files...", "Current iteration: " + i2 + " Progress: " + progress);
              publishProgress(progress);
              new File(dir, children[i2]).delete();
         }
    }

    return null;
}

edited Aug 15 '13 at 01:34

answered Aug 14 '13 at 22:00

Tonithy

2,300
1
20
20

1

About #1: No. The variable `i` is not used in the nested for-loop. There is only one directory that is being processed 100 times. – Vikram Aug 14 '13 at 22:02
`ProgressBar accepts a float`. I don't think so. [Link](http://developer.android.com/reference/android/widget/ProgressBar.html#setProgress%28int%29). – Vikram Aug 14 '13 at 22:04
@vikram I figured that out once I reached #3. I can't imagine a specific reason to read through that directory and arbitrary (100), but specific amount of times. – Tonithy Aug 14 '13 at 22:04
@vikram I'll fix that. Just realized I was thinking of something else. – Tonithy Aug 14 '13 at 22:05
Yes, I was wondering about the same thing. #1 and #3 kind of contradict each other. – Vikram Aug 14 '13 at 22:06
You should multiply by 100 first, otherwise you will get integer rounding problems. (If we assume `i2 = 10`, `children.length = 50`, then `(10/50) = 0`, and `0*100 = 0`, rather than the expected value of 20.) – Cat Aug 14 '13 at 22:13
I've removed the outer loop, but I'm still having trouble. Now it just shows 0%, then proceeds to the task completion dialog once it's done. I'm seeing no progress movement. – geek_sauce Aug 14 '13 at 23:57
@geek_sauce Log out 100 * i2 / children.length() right before calling publishProgress. What are you getting for that? – Tonithy Aug 15 '13 at 00:02
@Tonithy I feel dumb, but I'm not sure how to do that. Currently I've got Log.d("Tag", String.format("Iteration(%d), file has been deleted", i2)); – geek_sauce Aug 15 '13 at 00:16
@geek_sauce Check the edit, then you should see log messages in your console as it goes through that loop. – Tonithy Aug 15 '13 at 00:21
Now it's not deleting files, and it's not showing the loop output in the console. It just jumps right into completion mode. – geek_sauce Aug 15 '13 at 01:28
@geek_sauce Perhaps the cache directory is empty now? Do you keep repopulating it with data? – Tonithy Aug 15 '13 at 01:34
Yeah, I keep repopulating it. Currently 633 files, totaling 5MB. When I click the action button, it says files are deleted... but they're still there. They were previously deleting fine before the update you posted above. Btw, thank you for helping. – geek_sauce Aug 15 '13 at 03:07
Nevermind. I forgot to change the package name back after I pasted your code. It is working now. Thanks again! – geek_sauce Aug 15 '13 at 03:34
@geek_sauce lol, that'll do it. I haven't released com.popularapp yet so there wouldn't be any cache! Also, that's one argument for using constants instead of typing in strings like that... – Tonithy Aug 15 '13 at 03:40

Progress Bar isn't very accurate

1 Answers1