Skip to main content
added 79 characters in body
Source Link
alecxe
  • 17.5k
  • 8
  • 52
  • 93

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does
  • .format() formatting can be replaced with an f-string if on Python 3.6+
  • get_file is probably not the most descriptive name for this function, consider renaming it - get_full_file_path?..

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does
  • get_file is probably not the most descriptive name for this function, consider renaming it - get_full_file_path?..

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does
  • .format() formatting can be replaced with an f-string if on Python 3.6+
  • get_file is probably not the most descriptive name for this function, consider renaming it - get_full_file_path?..
added 121 characters in body
Source Link
alecxe
  • 17.5k
  • 8
  • 52
  • 93

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does
  • get_file is probably not the most descriptive name for this function, consider renaming it - get_full_file_path?..

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does
  • get_file is probably not the most descriptive name for this function, consider renaming it - get_full_file_path?..
Source Link
alecxe
  • 17.5k
  • 8
  • 52
  • 93

You can simplify several things:

  • if custom_data_folder == '': can become if not custom_data_folder:
  • if os.path.exists(default_path) is True can be just if os.path.exists(default_path)

Other notes:

  • it is questionable that you return "The file or path does not exist." string in case a path does not exist. Throwing an error would probably make more sense
  • make your default directory name a constant
  • you can avoid calculating the full default path if custom is provided
  • you can reduce code duplication and construct a path first, then check if it exist, raise an error if it does not and return if does exist

All the above things taken into account, here is the modified code:

import os


DEFAULT_DIRECTORY = 'data'


def get_file(): 
    """TODO: docstring"""
    custom_data_directory = input("Name of custom data directory - Hit ENTER to default: ")
    file_name = input("File name: ")

    directory = custom_data_directory or DEFAULT_DIRECTORY
    path = os.path.abspath(os.path.join(directory, file_name))
    
    if not os.path.exists(path):
        raise ValueError("Path '{path}' does not exist".format(path=path))
    
    return path

Some thoughts for further improvements:

  • you should probably create a better separation of concerns and have this function parameterized with file and directory names - moving user input related things from out of the function
  • create a docstring explaining what your function does