6
\$\begingroup\$

My Windows desktop is messy with many files of different types here and there, therefore I thought of writing a program to organize it, and other directories too, for me. Here is my code:

import os
import shutil


class File:
    """Represents a computer file.

    attributes: path, name, extension
    """
    def __init__(self, path):
        """Inintializes a File object.

        file_path: str
        """        
        if os.path.isfile(path):
            self.path = path
            self.name = os.path.basename(path)
            self.extension = os.path.splitext(self.path)[1].lower()
        else:
            raise FileNotFoundError("This is not a valid file")

    def is_document(self):
        """Returns whether file is a document."""
        extensions = ('.htm', '.docx', '.docm', '.doc', '.dotx',
                    '.dotm', '.pdf', '.xps', '.rtf',
                    '.txt', '.xml', '.odt', '.wps', '.accdb',
                    '.mdb', '.adp', '.xlsx', '.xlsm', '.xls',
                    '.xlsb', '.xltx', '.xltm', '.xlt', '.htm'
                    '.csv', '.prn', '.dif', '.ods', '.xlam',
                    '.slk', '.pptx', '.pptm', '.ppt', '.potx',
                    '.potm', '.pot', '.ppsx', '.ppsm', '.epub')
        return self.extension in extensions

    def is_photo(self):
        """Returns whether file is a photo."""
        extensions = ('.png', '.jpeg', '.jpg')
        return self.extension in extensions

    def is_media(self):
        """Returns whether file is a media file."""
        extensions = ('.mp3', '.mp4', '.flv', '.amr', '.mpg', '.avi', '.mov', '.webm', '.rm')
        return self.extension in extensions

    def is_executable(self):
        """Returns whether file is an exeutable."""
        return self.extension == '.exe'

    def is_archive(self):
        """Returns whether file is a compressed file."""
        extensions = ('.rar', '.zip')
        return self.extension in extensions

    def type(self):
        """Returns a string representing the type of file."""
        if self.is_document():
            return 'document'
        elif self.is_photo():
            return 'photo'
        elif self.is_media():
            return 'media'
        elif self.is_executable():
            return 'executable'
        elif self.is_archive():
            return 'archive'
        else:
            return 'other'

    def move(self, other):
        """Moves the file to a given directory."""
        Folder(other)

        try:
            shutil.move(self.path, other)

        except:       
            print('Was not moved successfully: ', self.path)
            print()


class Folder:
    """Represents a computer folder.

    attributes: path, name
    """
    def __init__(self, path):
        """Initializes a Folder object.

        path: str
        """

        if not os.path.isdir(path):
            raise FileNotFoundError(path + ' is not a valid directory')

        else:
            self.path = path
            self.name = os.path.basename(path)


    def is_empty(self):
        """Returns whether a folder is empty."""
        return os.listdir(self.path) == []


def dir_path(message):
    """Asks the user to enter a string specifing
    a directory.

    message: str

    Returns: str
    """    
    while True:
        dir_path = input(message)

        print('')

        if os.path.isdir(dir_path):
            return dir_path

        print('Not a valid directory. Try again.\n')


def move_files(source, target):
    """Moves files in a given directory to folders working as
    categories, based on their type."""
    Folder(source)
    Folder(target)

    categories = ('Document', 'Executable', 'Archive', 'Media', 'Photo', 'Other')
    create_folders(target, categories)

    for item in os.listdir(source):
        full_path = os.path.join(source, item)
        if os.path.isfile(full_path):
            file = File(full_path)
            category = os.path.join(target, file.type().title())

            print('Move: ', full_path)
            print()

            file.move(category)


def create_folders(directory, names):
    """Creates one or more folders in a given directory."""
    target = Folder(directory)

    try:
        for name in names:
            full_path = os.path.join(directory, name)
            os.mkdir(full_path)

    except FileExistsError:
        print(full_path, ': Exists already')


def main():            
    source = dir_path('Path of directory to organize: ')
    name = 'Directory Organizer'
    target = os.path.join(source, name)    
    create_folders(source, (name,))
    move_files(source, target)

    print('\nDone')


if __name__ == '__main__':
    main()

In addition to getting my code reviewed I have some questions regarding it:

  • Should I in create_folders use pass instead of printing a message?

  • I thought of writing a function that will categorize files existing in a given a directory based on their type. And will return a dictionary in which key is a string representing a category and value is a list of strings representing paths of files that belong to this category. Then another function will traverse this dictionary and move files. But I thought that this will just slow my code down. Will this be better?

  • Should I care about "access denied errors"?

  • I feel that move_files is not good enough. Am I right? If so how should it be refactored?

\$\endgroup\$
2
\$\begingroup\$

There is quite a lot of repetition in the way you determine the file type. Plus, this approach does not scale well. What if you would have a dictionary of file type names as keys and a frozenset of extensions as values which you would dynamically transform into a map between an extension and a file type:

TYPE_EXTENSIONS = {
    'document': frozenset(['.htm', '.docx', '.docm', '.doc', '.dotx',
                           '.dotm', '.pdf', '.xps', '.rtf',
                           '.txt', '.xml', '.odt', '.wps', '.accdb',
                           '.mdb', '.adp', '.xlsx', '.xlsm', '.xls',
                           '.xlsb', '.xltx', '.xltm', '.xlt', '.htm'
                           '.csv', '.prn', '.dif', '.ods', '.xlam',
                           '.slk', '.pptx', '.pptm', '.ppt', '.potx',
                           '.potm', '.pot', '.ppsx', '.ppsm', '.epub']),
    'photo': frozenset(['.png', '.jpeg', '.jpg']),
    'media': frozenset(['.mp3', '.mp4', '.flv', '.amr', '.mpg', '.avi', '.mov', '.webm', '.rm']),
    'executable': frozenset(['.exe']),
    'archive': frozenset(['.rar', '.zip'])
}

EXTENSION_TYPES = {value: key 
                   for value in values
                   for key, values in TYPE_EXTENSIONS.items()}

Now, the type() method would simply look like:

def type(self):
    """Returns a string representing the type of file."""
    return EXTENSION_TYPES.get(self.extension, 'other')

And you would not need all the is_*() methods anymore.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I'd go a step further and transform FILE_TYPES into a dictionary with the keys and values inverted. \$\endgroup\$ – 200_success Feb 27 '17 at 23:53
  • \$\begingroup\$ @200_success really good idea, I've adjusted the answer appropriately. Thanks! \$\endgroup\$ – alecxe Feb 28 '17 at 0:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.