1
\$\begingroup\$

I made a game requiring reaction time, and I just came here to see if there's anything I could improve on:

import datetime
import random
import time


points = {}
player_1 = input("Player one: ")
player_2 = input("Player two: ")
points[player_1] = 0
points[player_2] = 0
rounds = int(input("Rounds: "))
while rounds % 2 != 0:
    print("Sorry, you have to give me an even number!")
    rounds = int(input("Rounds: "))
turn = random.randint(0, 1)
for r in range(0, rounds):
    if turn == 0:
        print("It's {}'s turn!".format(player_1))
    else:
        print("It's {}'s turn!".format(player_2))
    time.sleep(0.5)
    print("Get ready..")
    time.sleep(random.randint(1,12))
    then = datetime.datetime.now()
    t = input("GO!! ")
    now = datetime.datetime.now()
    diff = then-now
    reaction_time = round(abs(diff.total_seconds()), 2)
    if turn == 0:
        points[player_1] += reaction_time
        turn += 1
    else:
        points[player_2] += reaction_time
        turn -= 1
    print("Reaction time: {} seconds".format(reaction_time))
winner = min([points[player_1], points[player_2]])
print("The winner is..")
time.sleep(0.5)
print(player_1+"!")
\$\endgroup\$
  • 2
    \$\begingroup\$ Not a review, but player_1 is currently the hardcoded winner. \$\endgroup\$ – Ludisposed Nov 22 '17 at 0:26
  • \$\begingroup\$ Welcome to Code Review! Are you using Python 3.x? \$\endgroup\$ – Phrancis Nov 22 '17 at 0:26
  • \$\begingroup\$ @Ludisposed whoops, i'll fix that \$\endgroup\$ – Sipher_ Nov 22 '17 at 3:46
2
\$\begingroup\$

Two things regarding redability:

  1. Use single blank lines to organize your code in logically related blocks. For example after the variable initializations, and before and after evry while or for loop. But don't take it too far, i. e. don't separate every if block or short inner loop within one big block.

  2. Use functions to abstract details. Organizing code blocks of 3 - 10 lines into functions with expressive names can greatly increase readability and maintainability, and make it easier to find bugs. The function name should be as short as possible, but as long as needed to still explain what it does and not use any unusual abbreviations. That way the main code would read like a story, while the technical implementation can easily be found when needed.

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

I'd probably use range(0, 2*rounds) instead of forcing the user to input an even number. "round" usually means 1 turn for each player. Also 0%2 == 0 so if users enter 0 they'll get a crash when the program skips the loop and drops down to winner = min([points[player_1], points[player_2]]) despite points[player_1] having not been defined yet.

Note: I've been writing a lot of python2.7 lately so this may have syntactic errors...

You could have unlimited players if you add some more loops.

points = {}
newplayer = input("Enter first player name: ")
points[newplayer] = 0
another = input("Add another player? Y/n")
while another != "n":
    newplayer = input("Enter first player name: ")
    points [newplayer] = 0
    another = input("Add another player? Y/n")
...

Then you can iterate over keys in points instead of having separate variables for player name, and there's no need for separate turn counting.

...
for i in range(0, 2 * rounds):
    for player in points.keys():
        print("It's {}'s turn!".format(player))
        time.sleep(0.5)
        print("Get ready..")
        time.sleep(random.randint(1, 12))
        then = datetime.datetime.now()
        t = input("GO!! ")
        now = datetime.datetime.now()
        diff = then-now
        reaction_time = round(abs(diff.total_seconds()), 2)
        print("Reaction time: {} seconds".format(reaction_time))
        points[player] += reaction_time
\$\endgroup\$
  • \$\begingroup\$ Thanks! I don't really want more than 2 players, because it's designed to be 2 players, but thank you for the other suggestions! \$\endgroup\$ – Sipher_ Nov 22 '17 at 3:48
1
\$\begingroup\$

I decided to implement mine for 2 players, by default, but if you want more then you can simply uncomment the int(input("Number of players: ")). Next, I am confused as to why you chose to use datetime, I removed that. time already has a built into it and it reduces an include.

Next, I noticed that you are measuring reaction time in hundredths of seconds. Most of the time, when you're talking about reaction times, (in my experience) people tend to use milliseconds, so I changed it to said unit. I modified a lot of your formats with f"". This is a Python 3 feature. It basically allows you to use .format without so much dress up.

A really big change was how I implemented your players. I used a dictionary instead, that way we can store all relevant information about a player in one variable. So, I have a list of players and each player has a name and points attribute. This makes our code significantly cleaner.

I reduced the number of seconds you wait while waiting to 'react'. I found 12 seconds to be a bit over kill, so I reduced it to 4 seconds.

Because of how I modified your data structure, I used they key keyword argument of the min function. I used it so I find the smallest based on points. Then we grab name. Making the full code: winner = min(players,key=lambda a: a['points'])['name'].

import random, time

num_of_players = 2#int(input("Number of players: "))
players = [{
            'name':input(f"Player {i+1}: "),
            'points':0
            } for i in range(num_of_players)]
rounds = int(input("Rounds: "))
for i in range(0,rounds):
    print("-"*20 + f"\nIt is now round {i}!\n" + "-"*20)
    for player in players:
        input(f"It is now {player['name']}'s turn.\nPress enter when ready.")
        time.sleep(.5)
        print("Get ready..")
        time.sleep(random.randint(1,4))
        then = time.time()
        t = input("GO!!")
        reaction_time = time.time()-then
        print(f"{reaction_time*1000:.0f} ms")
        player["points"] += reaction_time
winner = min(players,key=lambda a: a['points'])['name']
print("The winner is...")
time.sleep(0.5)
print(f"{winner}!")
\$\endgroup\$

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.