Issue Regarding The Reporting of Child Files In a Folder After They Are Deleted

This is a file class question. I’m working on an app (same as the last post on this forum I made) that is essentially a folder utility app. I’m just giving a bunch of operations to perform on a directory, and its subdirectories, to clean it up. One of them is “remove empty folders.” My algorithm isn’t the best as of right now, but I am having an issue. Let me set up a specific scenario:

Main Folder/Folder With One Item/Empty Folder

In this situation, Folder with One Item has contents until Empty Folder is recognized as empty and is moved to the trash via moveToTrach(), but when we come back to Folder With One Item and try to return its children, its still reports one file (even with hidden files disabled). Is this due to when the file is actually moved to the trash?

Here’s the class, which I’ve stripped down to just what’s needed for the issue.

class Directory
{
private:
    
    File fileHolder;
    String listOfFoldersRemoved;
    int numberOfFilesRemoved;
    bool iterateAgain;
    Array<File> filesArray;
    
    bool folderIsEmpty();
    void removeSingleEmptyFolder();
    
public:
    Directory();
    void removeAllEmptyFolders();
};

Implementation:

Directory::Directory()
{
    iterateAgain = false;
}

void Directory::removeAllEmptyFolders()
{
    // Scans files and directories recursively, but skips hidden files
    DirectoryIterator dirIter (File (mainPathway.getFullPathName()), true, "*",
                               File::findFilesAndDirectories + File::ignoreHiddenFiles);
    
    // Iterate and collect all files and store in filesArray
    while(dirIter.next())
    {
        filesArray.add(dirIter.getFile());
    }
    
    // Fix this algorithm
    // Test Commit
    
    do
    {
        iterateAgain = false;
        
        for(int i = 0; (i < filesArray.size()) && (iterateAgain == false); i++)
        {
            fileHolder = filesArray[i];
            
            if(fileHolder.isDirectory())
            {
                if(folderIsEmpty())
                {
                    iterateAgain = true;
                    filesArray.remove(i);
                    removeSingleEmptyFolder();
                }
            }
        }
    }
    while(iterateAgain);
    
    // Add remaining infomrmation to text string for textEditor
    listOfFoldersRemoved += "Completed: ";
    listOfFoldersRemoved += (String) numberOfFilesRemoved;
    listOfFoldersRemoved += " empty folder(s) moved to trash.\n\n";
}

bool Directory::folderIsEmpty()
{
    int numberOfFiles = fileHolder.getNumberOfChildFiles(File::findFilesAndDirectories +
                                                         File::ignoreHiddenFiles);
    // THIS IS WHERE ISSUE OCCURS, JUCE reports that a folder containing a recently
    // Deleted folder still has one item in it
    if(numberOfFiles != 0)
    {
        return false;
    }
    
    return true;
}


void Directory::removeSingleEmptyFolder()
{
    // Move file
    fileHolder.moveToTrash();

    // Add filename to list of removed files
    listOfFoldersRemoved += (numberOfFilesRemoved + 1);
    listOfFoldersRemoved += ": /";
    listOfFoldersRemoved += fileHolder.getRelativePathFrom(mainPathway.getFullPathName());
    listOfFoldersRemoved += "\n\n";

    // Increment number of files removed
    numberOfFilesRemoved++;
}

I’m still relatively new to programming, so I hopefully I’m not making too many mistakes here.

It’s probably failing to delete the folder. You should actually check the return value. And maybe try File::deleteRecursively() instead of moveToTrash()

Well, I really like the idea of things being moved to the trash. For now, it gives me an extra layer of security in case my algorithm isn’t perfect and moves something it shouldn’t. I’m still a bit confused. I tried two things. First, I tested the return value like you said, and it doesn’t seem to having an issue. Its returning true on the first attempt, though sometimes it doesn’t seem to be actually doing this. I then decided to infinitely loop the attempts to move it to trash, just in case it wasn’t being moved the first time:

While (! fileHolder.moveToTrash);

This produced the same output as I had before, it things weren’t always being moved. Strange.

I tried the recursive delete at you suggested and my algorithm is working 100 percent perfectly. However, now files are instantly deleted and I would need to implement some sort of “are you sure you want to delete?” I really liked the trashcan way, wish I could get it to work.

This is the output with the recursiveDelete() being called, which I should switch to just delete, as its only acting on the last folder in the chain and isn’t deleting anything else. This is a perfect result for me, only wish it was being moved to trash:

This is the output when calling moveToTrash(), items try multiple times and I have to run the remove operation multiple times to catch everything. This is with the: while(! fileHolder.moveToTrash); implementation.

Have you debugged into the move-to-trash function to see if there are any clues? IIRC it just calls an OS function to do that so if it doesn’t work, that may be beyond our control…

I haven’t. I have just been skipping your moveToTrash() function when debugging, but I’ll take a look into it. However, I’m only a year 2/3 student in comp science, so your code may be way above my head. I’ll give it a look. Also, there’s no reason not to think my code isn’t at fault, I’m not a pro by any means, but my algorithm does to be working flawlessly when delete() is called.

Anyone else have any other alternatives for this?

I think the actual move-to-trash may happen on another thread via the OS. I seem to remember hitting this at some point. E.g., calling File::exists() returns true, move-to-trash, and calling File::exists() sometimes return still returns true.

Yep. This seems to be exactly my problem. The same issue occurs when using the moveFileTo() function, as I tried to move all files to a temp folder, then move that whole folder over to trash. I’ve been RACKING my head trying to find another way to get these files to the trashcan. Deleting them instantly isn’t something I want to do at all and I don’t entirely want to rewrite half of my application to allow the user to see, open, and delete all folders caught, before deleting, as that seems like a ton of work for a workaround.

Well, you could moveToTrash() and then loop, waiting until File::exists() returns false?

I think that was the solution I used, with a loop counter to give up after few tries… just in case

EXCELLENT! This is working so far! Thanks a ton for the tips guys! This was driving me nuts! My code:

while(Base::getFileHolder().exists())
    {
        Base::getFileHolder().moveToTrash();
    }

Output:

Glad it’s working, but like Martin suggested, you’re going to want a time-out on that!

With the timeout, I assume that in some cases, it will miss a file here and there… correct? A statement of that would be something I would put in release notes… “Possibility of not deleting an empty exists” or something…

Also, hard to know when to draw the cutoff…

Without a timeout, trying to delete a read-only file would be an infinite loop.

Ahhh, gotcha! Makes total sense. I never thought of that. Thanks again guys. This really saved me and made my day! :slight_smile:

Maybe I should rename this thread to better reflect the issue for future reference, now that we tracked it down, and its not what was suggested in the title.

One last thing, I did a cout statement to the console to get a feel for how many loops were happening on some of the files. Here’s the results for 21 items movedToTrash()

38

4

9

14

7

8

6

7

7

9

10

11

11

9

12

11

6

6

6

6

9

Putting a Thread::yield() in the loop might help a little. Or even some smart Thread::sleep() that kicks in after the first failure.

Might have to try this. I tested the last solution it oringally on my 2015 MacBook Air 11 inch running OS 10.12.3 and it worked perfectly, but moving over to my 2011 21.5 inch iMac running OS 10.10.5, its back to failing with half of the files. So compatibility is an issue.

You are essentially continuously asking the OS to move the file to trash until it finally goes.

Perhaps you could be easier on the underlying system if you invoke the moveToTrash() only once, before the while, and then you just yield() inside the loop until the file is gone (or the timeout expires).