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\$
  • \$\begingroup\$ The code does work, I was just concerned I may run into difficulties later on as I develop it \$\endgroup\$ – helpmepls Dec 11 '15 at 23:01
  • \$\begingroup\$ I used python 3.4 \$\endgroup\$ – helpmepls Dec 11 '15 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\$ – 200_success Dec 11 '15 at 23:03
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))
|improve this answer|||||
\$\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).

|improve this answer|||||
\$\endgroup\$
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

|improve this answer|||||
\$\endgroup\$
  • 2
    \$\begingroup\$ No need for myFile.close(). \$\endgroup\$ – Barry Dec 11 '15 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').

|improve this answer|||||
\$\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();
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ To my knowledge, using % for formatting is discouraged, but not yet deprecated. \$\endgroup\$ – 200_success Dec 11 '15 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 '15 at 23:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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