3
\$\begingroup\$

The text file is laid out like this: first name, last name, wins, losses

dex,kop,5,6

desder,jok,0,11

aslan,vert,11,0

jax,sas,2,9

buy,guy,1,10

Code:

def points():

    template = "|{0:<30}|{1:<30}|{2:<30}|{3:<30}|{4:<30}"
    lol = template.format("1st name: ","2nd Name: ", "won: ", "lost: ","points: ")

    print(lol)

    with open(r'prac2.txt', 'r') as file:

        for line in file:

            data = line.strip().split(',')

            if int(data[2]) >= 1:

                poin = int(data[2]) * 3

                data.append(poin)

                print(template.format(data[0], data[1], data[2], data[3], data[4]),'\n')

        file.close()

points()

The program should only display players that have won at least 1 game and calculate their points, a win is equal to 3 points.

Any suggestions as to how to improve the code any would be great.

\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

Since you are using all the elements of data, You can turn:

print(template.format(data[0], data[1], data[2], data[3], data[4]))

into this:

print(template.format(*data))

See here for some details.

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

A really simple but useful tip; check the name of your vars. Even when you are just doing a simple exercise.

What is lol and poin, the same with the name of your method, what does points mean? Add, remove, update, print points?

Think about someone without time that is looking your code and is trying to understand what are you doing.

Maybe something like print_leaderboard can be easier to read/understand. Same with lol versus leaderboard_header. Etc..

This is a simple thing, even when you don't have time. Because when you don't have time is when all those little things helps!

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

I would also switch to using csv module and the DictReader that will help make the code more readable by having column names assigned to each of the columns.

I would also improve on variable names - for instance, since each row in the input file represents a player, let's call it a player. The third column corresponds to the number of wins - let's call it wins. lol variable name does not make much sense, I would name it header since this is an output "table" header.

Other notes:

  • you don't need to close the file explicitly - the whole point of using the with context manager is to let Python close the file implicitly and safely
  • execute the function from under the if __name__ == '__main__': to avoid it being execute on import

Here is the code with the above and some other changes applied:

import csv


HEADERS = ["first_name", "last_name", "wins", "losses"]
TEMPLATE = "|{first_name:<30}|{last_name:<30}|{wins:<30}|{losses:<30}|{points:<30}"


def report_points(filename):
    """
    Reads the input file with players data. 
    Reports players with at least a single win calculating the total number of points.
    """
    header = TEMPLATE.format(first_name="1st name: ",
                             last_name="2nd Name: ",
                             wins="won: ",
                             losses="lost: ",
                             points="points: ")
    print(header)

    with open(filename, 'r') as file:
        reader = csv.DictReader(file, HEADERS)
        for player in reader:
            wins = int(player["wins"])
            if wins >= 1:
                print(TEMPLATE.format(**player, points=wins * 3), '\n')

if __name__ == '__main__':
    report_points(r'prac2.txt')

If you are on Python3.6, I would also look into using f-strings instead of formatting via .format().

\$\endgroup\$

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.