Menu

#5 DirWatcher has errors in iteration in notifyDirChanged()

open
nobody
None
5
2009-05-14
2009-05-14
Anonymous
No

This code appears to be previously untested, and with the problem I described in bug 2791735, the problem could possibly go unnoticed. When the aforementioned bug is fixed, notifyDirChanged() is called, exposing the problem that the various Vectors are being iterated incorrectly. For example, the developer was iterating similar to this:

for (int i = collection.size(); i >= 0; --i) {
doSomething(collection.itemAt(i));
}

I think the intent was to pre-decrement the variable to make the indexing look cleaner, but that is obviously not how the post-condition of a for loop works. This is my corrected listing for the method, but I am using a "for each" loop, but that might be undesirable for some developers that need more backward compatibility:

private void notifyDirChanged(File dir) {
WatchedDir w = (WatchedDir) dirs.get(dir);
if (w == null)
return;

// get and sort
String[] files = dir.list();
StringSortable.sortArray(files, SelectionSorter.get());

Vector dels = w.whatsDeleted(files);
Vector adds = w.whatsCreated(files);

// deletions first, since the consequences of thinking
// that a deleted file still exists are more dire
for (Object fileName : dels) {
File deld = new File(dir, (String) fileName);

// notify everyone
for (Object listener : w.who) {
((DirChangedListener) listener).handleFileDeleted(this, dir,
deld);
}
}

// additions next
for (Object fileName : adds) {
File addd = new File(dir, (String) fileName);

// notify everyone
for (Object listener : w.who) {
((DirChangedListener) listener).handleFileCreated(this, dir,
addd);
}
}

// note the new contents of the directory
w.files = files;
}

Discussion


Log in to post a comment.