2
\$\begingroup\$

I've written a bunch of functions that deal with handling files on a Linux system. I'm looking for critique on the whole class, in particular:

  • Exception handling
  • Efficiency
  • Style

I'd also like to discuss the practice on marking all these methods as static and using a class merely as namespace.

As far as I'm aware, all methods do what they should. Backwards compatibility with Python 2.x is a nonissue.

import os


class FileIO:

    @staticmethod
    def find_subdir(filename):

        try:
            for root, dirs, files in os.walk(os.getcwd()):
                if filename in files:
                    return os.path.relpath(root)

        except IOError as e:
            print(filename + " not found: " + str(e))


    @staticmethod
    def find_in_subdir(filename):

        try:
            for root, dirs, files in os.walk(os.getcwd()):
                if filename in files:
                    return os.path.join(root, filename)

        except IOError as e:
            print(filename + " not found: " + str(e))

    @staticmethod
    def find_file(filename, directory):

        try:
            for root, dirs, files in os.walk(directory):
                if filename in files:
                    return os.path.join(root, filename)

        except IOError as e:
            print(filename + " not found: " + str(e))

    @staticmethod
    def copy_file(source, destination):

        try:
            copyfile(source, destination)

        except Exception as e:
            print("Couldn't copy file: " + str(e))

    @staticmethod
    def check_file(path):

        try:
            os.path.exists(path)
            return True

        except Exception as e:
            print("File not found or not accessible: " + str(e))
\$\endgroup\$
2
\$\begingroup\$
  • Docstrings: You should include a docstring at the beginning of every method/class/module you write. This will allow documentation to identify what your code is supposed to do. PEP-8 Docstring Conventions
  • String formatting: Concatenating variables and strings can become messy. Instead, use a format string f"" to directly include variables into your string, without having to type cast them or alter them in any way to allow them to be added with strings.
  • Use specific exceptions (when you can): Catching a general Exception isn't always the best idea. If you're preforming a task that generates only a specific kind of exception, catch that one.

I'd also like to discuss the practice on marking all these methods as static and using a class merely as namespace.

In this context, it's perfectly fine. All of these methods have the functionality of being used and called as a static method.

Updated Code

"""
Module Docstring:
A description of your program goes here
"""

import os

class FileIO:
    """
    Namespace class for handling files on a Linux system
    """
    @staticmethod
    def find_subdir(filename):
        """
        Finds the subdirectory from `file_name`

        :param filename: Name of the file (duh)

        """
        try:
            for root, dirs, files in os.walk(os.getcwd()):
                if filename in files:
                    return os.path.relpath(root)

        except IOError as e:
            print(f"{filename} not found: {e}")

    @staticmethod
    def find_in_subdir(filename):
        """
        Finds a file in a subdirectory

        :param filename: Name of the file (duh)

        """
        try:
            for root, dirs, files in os.walk(os.getcwd()):
                if filename in files:
                    return os.path.join(root, filename)

        except IOError as e:
            print(f"{filename} not found: {e}")

    @staticmethod
    def find_file(filename, directory):
        """
        Find a file in a directory

        :param filename: Name of the file to find
        :param directory: Directory to find the file

        """
        try:
            for root, dirs, files in os.walk(directory):
                if filename in files:
                    return os.path.join(root, filename)

        except IOError as e:
            print(f"{filename} not found: {e}")

    @staticmethod
    def copy_file(source, destination):
        """
        Creates a copy of a file at `source`, and places it at `destination`

        :param source: Path to the file to be copied
        :param destination: Path where the copied file should be placed

        """
        try:
            copyfile(source, destination)

        except Exception as e:
            print(f"Couldn't copy file: {e}")

    @staticmethod
    def check_file(path):
        """
        Checks if the passed file exists

        :param path: Path to the file

        """
        try:
            os.path.exists(path)
            return True

        except FileNotFoundError as e:
            print(f"File not found or not accessible: {e}")


\$\endgroup\$

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.