\$\begingroup\$
\$\endgroup\$
Part of my code:
def get_file():
custom_data_folder = input("Name of custom data directory - " +
"Hit ENTER to default: ")
file_name = input("File name: ")
default_path = os.path.abspath(os.path.join('data', file_name))
custom_path = os.path.abspath(os.path.join(custom_data_folder, file_name))
if custom_data_folder == '':
return(default_path if os.path.exists(default_path) is True
else "The file or path does not exist.")
else:
return(custom_path if os.path.exists(custom_path) is True
else "The file or path does not exist.")
My script assumes a file is in a directory called "data", but I wanted to provide an option of a entering a custom directory where a file could be.
It works, but is there a better way to go about doing what this function is trying to accomplish?
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Aug 1, 2017 at 17:04
1 Answer 1
\$\begingroup\$
\$\endgroup\$
2
You can simplify several things:
if custom_data_folder == '':
can becomeif not custom_data_folder:
if os.path.exists(default_path) is True
can be justif 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 anf-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
?..
answered Aug 1, 2017 at 17:20
-
\$\begingroup\$ Interesting advice, thank you. In regards to your "separation of concerns" point, I agree that they should be shifted out of the function. After reading your answer, I think perhaps argparse could help in that. Thank you for all the advice. Your answer has given me much to work with. \$\endgroup\$Walid Mujahid وليد مجاهد– Walid Mujahid وليد مجاهد2017年08月01日 18:19:29 +00:00Commented Aug 1, 2017 at 18:19
-
\$\begingroup\$ @WalidMujahid ah, yes,
argparse
is a good idea - you can even use custom file and directory argument types there. Thanks. \$\endgroup\$alecxe– alecxe2017年08月01日 18:20:36 +00:00Commented Aug 1, 2017 at 18:20
lang-py