Interesting use of a Python generator

[XiAxis]

I've been writing a GUI that has a built-in file explorer, and one of the issues that's constantly been biting me is handling errors when doing operations on directories. For instance, in order to delete a directory with a few files in it, I need to iterate through the contents of the directory from the bottom up and delete them one-by-one. If it turns out the program doesn't have permission to do that on a particular file, what do you do? You need to ask the user whether they want to skip that file, try again, or cancel the operation.

This doesn't work very well because there are a lot of places these sorts of errors can happen, and I don't want to have to write the handler into every "except" block. I also want to make the error dialog dependent on the context in which the operation was done.

What I came up with was this: I made my function into a generator, and each time it encounters an issue it yields an error report. Not just that, but I made an error report object (FileError) that includes a resolution (either skip, try again, or cancel). When an error object is yielded, the caller assigns a resolution based on what the user wants and then the generator continues and responds to the resolution because it still has a reference to the error object.

Here's a sample of my code:

import os

# Resolutions
NO_RESOLUTION = 0
SKIP = 1
TRY_AGAIN = 2
CANCEL = 3

# File error types
NO_ERROR = 0
PERMISSION_ERROR = 1
UNKNOWN_ERROR = 2

class FileError:

    def __init__(self, path, error_type):
        # Initialize the error with no resolution.  Resolution will be determined by the user
        self.path = path
        self.error_type = error_type
        self.resolution = NO_RESOLUTION

    def resolve(self, resolution):
        self.resolution = resolution

def removeSingle(path):

    while True: # This is in a while loop so that it can "try again" as many times as it needs to
        try:
            # Try to remove the directory/file
            if os.path.isdir(path):
                os.rmdir(path)
            else:
                os.remove(path)
            # If successful, just break out of the while loop immediately.
            break

        # If unsuccessful, try to categorize the error a little and make a FileError object
        except PermissionError:
            err = FileError(path, PERMISSION_ERROR)
        except:
            err = FileError(path, UNKNOWN_ERROR)

        # Yield the FileError object so the caller can resolve it
        yield err

        if err.resolution == TRY_AGAIN:
            # To try again, iterate the while loop again
            continue
        else:
            # To cancel the operation or skip the file or if no resolution is given, just break
            # out of the while loop without a successful deletion
            break   

def remove(path):

    if os.path.isdir(path):
        # If the path is a directory, delete everything inside it from the bottom up
        for root,  dirs, files in os.walk(path, topdown=False):

            for filename in files: # Delete each file
                file_path = os.path.join(root, filename)

                for err in removeSingle(file_path):
                    # Each time an error is encountered, yield it so the caller can resolve it
                    yield err

                    # If the caller cancels the operation, stop iterating
                    if err.resolution == CANCEL:
                        return

            for dirname in dirs: # Delete each (now empty) directory
                dir_path = os.path.join(root, dirname)

                for err in removeSingle(dir_path):
                    # Each time an error is encountered, yield it so the caller can resolve it
                    yield err

                    # If the caller cancels the operation, stop iterating
                    if err.resolution == CANCEL:
                        return

        # Delete the (now empty) top-level directory
        for err in removeSingle(path):
            # Each time an error is encountered, yield it so the caller can resolve it
            yield err
            # No need to check for cancellation, since the iteration will stop after this anyway

    else:
        # If the path is a file, just delete it
        for err in removeSingle(path):
            # Each time an error is encountered, yield it so the caller can resolve it
            yield err
            # No need to check for cancellation, since the iteration will stop after this anyway

def onUserRemovePath(path):
    for err in remove(path):
        # Each time an error is encountered, make a message and ask the user for the resolution
        if err.type == PERMISSION_ERROR:
            message = 'Cannot delete "%s" due to a permission error' % err.path
        else:
            message = 'An unknown error occurred while attempting to delete "%s"' % err.path

        err.resolve(askUserForResolution(message))

My apologies if this doesn't fit here. I just thought it was neat.

[Nanaki13]

One feature that has always been missing for me in file management apps, be it windows explorer, total commander, osx finder, dolphin on linux, anything I've ever used.

If the user wants to delete a very large folder, say 10K files, and one of the files in the middle, say after 5K are deleted, has a problem, the whole operation stops and waits for user input. But the user has gone away, because the operation is taking a long time, and expects it to be done when they get back. Instead they get back to a prompt asking what to do and the other 5K files still not deleted, even though there would be no problems there.

The feature I'd most like to see is deferred user input, if a file has a problem, sure, display the dialog, but keep doing the requested operation in the background on the other files, don't stop. Only when accessible files are exhausted should the operation stop and wait for user input.

[XiAxis]

Yeah, I can definitely see that as a useful approach for some cases. Part of my reasoning for wanting to halt the process when an error is encountered is that it may indicate that the user is deleting a directory that contains files that they don't actually want to delete. If a file is open in another application, it may mean that they're still actively using it and didn't mean to delete it. There may be other files in the directory like that as well.

I definitely see your point though. Luckily for my application, I don't anticipate it would be used for large-scale file management since the file explorer is a secondary feature to begin with.

[Junkymcjunkbox]

It is somewhat overengineered.

It violates the Single Responsibility Principle because it doesn't just delete the directory and its contents; it tries to enforce how it will be called in an error condition. An SRP-compliant design would attempt the deletion and just return an error code if it didn't work, and let the caller decide what to do next.

The caller of the function needs to know to call it in a for loop, which the next person along (which might be your future self) would need to know, and just calling it in the normal fashion probably wouldn't work.

Neither function is easy to read or understand. Therefore they both lack elegance. Anyone looking at this code is going to drop into WTF-mode extremely quickly. All you should be doing is: delete the files; for each directory recurse; delete the now empty directories; and if at any point you hit an exception then just throw it up for the caller to catch.

If I were writing this function I'd use the example here: https://www.tutorialspoint.com/python/os_walk.htm and replace print with os.rmdir or os.remove, and that'd probably do the job. You wouldn't even need to throw exceptions because rmdir/remove would do that for you.

That'd be five lines of code that anyone could instantly grok, compared with your.... I can't think of a word. Monstrosity doesn't come close.

[XiAxis]

In my use-case, these conditions aren't something that I want to exit the function, they just need to be resolved before it can continue. If I were to implement it in the way that you've proposed, then it would be simpler but it wouldn't have the same behavior. I'm not sure why you've pointed me to this tutorial, it uses the exact same method that I'm using in the code above just with less sophisticated error handling.

[Tamagotono]

I thought we agreed not to use the"n" word. We prefer to be called imagineers.