current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

The following function is basically asking the user to input a file name for a file that the calling program is going to do something with. If the user enters an invalid file name, I want the program to loop back and give them another chance. The code works, but is this the proper way to accomplish this? Also, are there other errors that could get triggered that I should deal with?

def get_filename(file_type):
    while True:
        print('enter ' + file_type + ' filename: ')
        filename = input()
        print(filename)
        try:
            with open(filename, 'r') as f:
                my_file = f.read()
            return my_file
        except FileNotFoundError:
            print('No such file.  Check file name and path and try again.')


x = get_filename('TEMPLATE')
print(x)
share|improve this question

1 Answer 1

up vote 2 down vote

is this the proper way to accomplish this?

Yes; looping is the best approach for this type of functionality (see e.g. Asking the user for input until they give a valid response).

are there other errors that could get triggered that I should deal with?

There are reasons other than not finding the file that opening and reading the file could fail; in this case, I would probably catch the more general OSError to handle all of these. I would also restructure slightly to minimise the code in the try block:

try:
    with open(filename) as f:  # note that 'r' is the default mode
        my_file = f.read()
except OSError:
    print('Error accessing file. Check file name and path and try again.')
else:
    return my_file

A few other minor notes:

  • You should consider adding a docstring, explaining what the function does and how to use it;
  • Currently, the file_type is only used for display purposes - consider adding checks that the user has selected an appropriate file;
  • Returning the string content of the file means you have to read the whole thing into memory up-front, which may not be ideal with larger files; and
  • I would use str.format rather than + concatenation, and just pass the text straight to input as the prompt argument rather than having a separate print call.
share|improve this answer
    
Thanks @jon. concerning "read the whole thing into memory up-front" is there a better method? am I better off just reopening the file later in my program? –  Christopher Pearson Mar 5 at 14:04
    
for the string format, is it acceptabe to just say: print('enter %s filename' % file_type)? –  Christopher Pearson Mar 5 at 14:10
1  
@ChristopherPearson it depends what you're doing with the file. If you want to handle large files, I would suggest using os.path.isfile to check it in get_filename, and returning only the valid filename. Then you can open the file and iterate over it line-by-line at the appropriate point. On string formatting, I see % as somewhat old-fashioned/unpythonic vs. str.format, but it's not actually deprecated. –  jonrsharpe Mar 5 at 14:16
    
Is this the correct usage?: filename = input('enter {!s} filename: '.format(file_type)) –  Christopher Pearson Mar 5 at 16:24
1  
yes, sorry it does. I was just making sure it was the proper way. Thanks again! –  Christopher Pearson Mar 5 at 16:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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