6
\$\begingroup\$

I need to be able to store results of a game in a file with names and high scores where I will eventually want to review the file and sort it alphabetically, etc. I'm unsure if I have the correct way of storing the results to be able to do this.

class_number = input('Please enter your class number: ')

from datetime import datetime 
now = datetime.now()
time = ('%s/%s/%s %s:%s' % (now.day, now.month, now.year, now.hour, now.minute))

userscore = (user_name, ' ', str(total_score),' ', time, '  ')

if(int(class_number) == 1):
    with open('class1.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()

elif(int(class_number) == 2):
    with open('class2.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()

elif(int(class_number) == 3):   
    with open('class3.txt', 'a') as myFile:
        myFile.writelines(userscore)
        myFile.close()
\$\endgroup\$
3
  • \$\begingroup\$ The code does work, I was just concerned I may run into difficulties later on as I develop it \$\endgroup\$
    – helpmepls
    Dec 11, 2015 at 23:01
  • \$\begingroup\$ I used python 3.4 \$\endgroup\$
    – helpmepls
    Dec 11, 2015 at 23:02
  • \$\begingroup\$ On the other hand, if you expect the .txt files to be rewritten in alphabetical order, and you are asking how to accomplish that… that's a Stack Overflow question, because you haven't implemented it yet. \$\endgroup\$ Dec 11, 2015 at 23:03

5 Answers 5

7
\$\begingroup\$

Your date format is unconventional, because the minutes are not zero-padded. You could use %02d instead of %s, but an even better solution would be to use now.strftime('%d/%m/%Y %H:%M') or '{:%d/%m/%Y %H:%M}'.format(now).

PEP 8 says that you should put import statements at the top, for maintainability.

Calling open() in the context of a with block, as you have done, is good practice, as it means it will be automatically closed when exiting the block. Therefore, you should eliminate the myFile.close() calls.

The three output blocks should be combined into one.

from datetime import datetime

now = datetime.now().strftime('%d/%m/%Y %H:%M')
class_number = input('Please enter your class number: ')

if class_number in ['1', '2', '3']:
    with open('class{}.txt'.format(class_number), 'a') as f:
        f.writelines((user_name, ' ', str(total_score), ' ', now, '  '))

Contrary to its name, writelines() does not actually add line termination characters to the output. If you intend to append multiple records to a file, it would be better to put each record on a separate line. In that case, consider using a csv.writer with delimiter=' ' to output tabular data.

import csv

…

if class_number in ['1', '2', '3']:
    with open('class{}.txt'.format(class_number), 'a') as f:
        writer = csv.writer(f, delimiter=' ')
        csv.writerow((user_name, total_score, now))
\$\endgroup\$
4
\$\begingroup\$

You should use a date format with the year on front, then month followed by day - all filled up with leading zeros. The resulting string sorts correctly when sorted lexically (as text).

\$\endgroup\$
1
3
\$\begingroup\$

Do not overuse if, instead write:

with open('class{}.txt'.format(class_number), 'a') as myFile:
    myFile.writelines(userscore)
    myFile.close() # Not necessary as Barry noted

The class number 1 will be saved in class1.txt, the number 2 in class2.txt...

About str.format

\$\endgroup\$
1
  • 2
    \$\begingroup\$ No need for myFile.close(). \$\endgroup\$
    – Barry
    Dec 11, 2015 at 23:39
2
\$\begingroup\$

I second most of the other answers; however, as you want to sort by name, find high scores, and whatnot, you should seriously consider writing it all to a sqlite table instead.

Also, your date format string is not suitable for sorting. You should use .strftime('%Y-%m-%d %H:%M').

\$\endgroup\$
1
\$\begingroup\$

There are some improvement to be considered:

  • Convert class number into an int directly – If you use int(input('Please enter... ')) it is stored directly as an int
  • Use str.format instead of % - The latter is to be removed at some point in time, so you are better of using stuff like 'class{}.txt'.format(class_number), and similar for the date time string. Although the latter can also be written as: datetime.now().strftime("%d/%m/%Y %H:%M") according to datetime.strftime
  • Group code in functions – It is a good practice to keep your code within functions, i.e. a add_highscore() function, which in turn is called from if __name__ == '__main__':
  • Put imports at the very top – Don't hide imports within the code, put them right up there at the top
  • When you use with ... there is no need to close the file – The close() is implied when you leave the with block
  • Your script only does one line? – Your script only adds one line to the file, so to add another high score you need to run it once more. You might want to look into loop constructs like a while loop.
  • Where does your total_score and user_name come from? – This just pops out of nowhere? Where does it come from? Are you presenting just a part of your code here? I've included code in my refactored function to ask for it (without much error handling) if it not presented as part of function call

After applying all of these comments, here is my untested refactored code:

from datetime import datetime 

def add_highscore(user_name=None, class_number=None, total_score=None):
    """Adds the high score to corresponding classX.txt high score file."""

    if not class_number:
        class_number = int(input('Please enter your class number: '))

    if not user_name:
        class_number = input('Please enter your user name: ')

    if not total_score:
        class_number = int(input('Please enter your total score: '))

    if 1 <= class_number <= 3:
        with open('class{}.txt', 'a') as output_file:
            output_file.writelines('{} {} {}\n'.format(user_name, total_score, datetime.now().strftime("%d/%m/%Y %H:%M")))


if __name__ == '__main__':
   add_highscore();
\$\endgroup\$
2
  • \$\begingroup\$ To my knowledge, using % for formatting is discouraged, but not yet deprecated. \$\endgroup\$ Dec 11, 2015 at 23:35
  • \$\begingroup\$ @200_success, that is why I used the vaguely formed phrase of "to be removed at some point in time" \$\endgroup\$
    – holroy
    Dec 11, 2015 at 23:37

Your Answer

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

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