1
\$\begingroup\$

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
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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?..
answered Aug 1, 2017 at 17:20
\$\endgroup\$
2
  • \$\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\$ Commented 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\$ Commented Aug 1, 2017 at 18:20

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.