8
\$\begingroup\$

I created a dice rolling game where the outcome shows the highest sum of rolls per player. As you can see from the if/elif statements, it's not very eloquent. I do not want someone to rewrite those statements into something more efficient. I would like to know if there is a better way of writing it and what that suggestion would be?

import random

total_rolls = []
total_rolls_for = 0
player_one = []
player_two = []
player_three = []
player_four = []
max_total = []
max_total_for = []


while len(player_one) < 5:
    player_one.append(random.randint(1,12))
    if len(player_one) == 5:
        print("Player one rolled:", player_one)

while len(player_two) < 5:
    player_two.append(random.randint(1,12))
    if len(player_two) == 5:
        print("Player two rolled:", player_two)

while len(player_three) < 5:
    player_three.append(random.randint(1,12))
    if len(player_three) == 5:
        print("Player three rolled:", player_three)


while len(player_four) < 5:
    player_four.append(random.randint(1,12))
    if len(player_four) == 5:
        print("Player four rolled:", player_four)

         
print("The sum of p1 is:", sum(player_one))
print("The sum of p2 is:", sum(player_two))
print("The sum of p3 is:", sum(player_three))
print("The sum of p4 is:", sum(player_four))



if sum(player_one) > sum(player_two) and sum(player_one) > sum(player_three) and sum(player_one) > sum(player_four):
    print("aPlayer one has a higher score with:",sum(player_one))
elif sum(player_two) > sum(player_one) and sum(player_two) > sum(player_three) and sum(player_two) > sum(player_four):
    print("aPlayer two has a higher score with:",sum(player_two))
elif sum(player_three) > sum(player_one) and sum(player_three) > sum(player_two) and sum(player_three) > sum(player_four):
    print("aPlayer three has a higher score with:",sum(player_three))
elif sum(player_four) > sum(player_one) and sum(player_four) > sum(player_two) and sum(player_four) > sum(player_three):
    print("aPlayer four has a higher score with:",sum(player_four))


elif sum(player_one) == sum(player_two) and sum(player_one) > sum(player_three) and sum(player_one) > sum(player_four):
    print("Player one and two has a higher score with:",sum(player_one))
elif sum(player_two) == sum(player_one) and sum(player_two) > sum(player_three) and sum(player_two) > sum(player_four):
    print("Player two and player one has a higher score with:",sum(player_two))
elif sum(player_three) == sum(player_one) and sum(player_three) > sum(player_two) and sum(player_three) > sum(player_four):
    print("Player three and player one has a higher score with:",sum(player_three))
elif sum(player_four) == sum(player_one) and sum(player_four) > sum(player_two) and sum(player_four) > sum(player_three):
    print("Player four and player one has a higher score with:",sum(player_four))

elif sum(player_two) == sum(player_three) and sum(player_two) > sum(player_one) and sum(player_two) > sum(player_four):
    print("Player two and three has a higher score with:",sum(player_two))
elif sum(player_two) == sum(player_four) and sum(player_two) > sum(player_three) and sum(player_two) > sum(player_one):
    print("Player two and player four has a higher score with:",sum(player_two))
elif sum(player_three) == sum(player_four) and sum(player_three) > sum(player_two) and sum(player_three) > sum(player_one):
    print("Player three and player four has a higher score with:",sum(player_three))


    
elif sum(player_four) == sum(player_one) == sum(player_two) and sum(player_four) > sum(player_three):
    print("Player four and player one and player two has a higher score with:",sum(player_four))
elif sum(player_three) == sum(player_one) == sum(player_two) and sum(player_one) > sum(player_four):
    print("Player three and player one and player two has a higher score with:",sum(player_one))
elif sum(player_four) == sum(player_three) == sum(player_two) and sum(player_four) > sum(player_one):
    print("Player four and player three and player two has a higher score with:",sum(player_four))
elif sum(player_one) == sum(player_three) == sum(player_four) and sum(player_four) > sum(player_two):
    print("Player one and player three and player four has a higher score with:",sum(player_four))
elif sum(player_one) == sum(player_four) == sum(player_three) == sum(player_two):
    print("all players have the same value:", sum(player_one))
\$\endgroup\$
6
\$\begingroup\$

To cut down on the long chain of if/elif statements, you could try something like this:

sum_p1 = sum(player_one)
sum_p2 = sum(player_two)
sum_p3 = sum(player_three)
sum_p4 = sum(player_four)
highest_score = max(sum_p1, sum_p2, sum_p3, sum_p4)
highest_scoring_players = []
if sum_p1 == highest_score:
    highest_scoring_players.append('Player 1')
if sum_p2 == highest_score:
    highest_scoring_players.append('Player 2')
if sum_p3 == highest_score:
    highest_scoring_players.append('Player 3')
if sum_p4 == highest_score:
    highest_scoring_players.append('Player 4')
print(' and '.join(highest_scoring_players) + ' has a higher score with:', highest_score)

Note that we can't use elif because it could be the case that both Player 1 and 2 have the highest score.

There's still a dissatisfying amount of repetition in the above example, so I'd suggest rewriting your code to have a list of players rather than four individual player variables. I've given an example of rewriting your program with this new approach.

import random

NUM_PLAYERS = 4
# Instead of storing each player's list of rolls, we can just store the score
# in a list
player_scores = []
# Create four players (range loops through integers 1 <= i < 5)
for i in range(1, NUM_PLAYERS + 1):
    # Generate the 5 dice rolls for player i
    rolls = []
    while len(rolls) < 5:
        rolls.append(random.randint(1, 12))
    print(f"Player {i} rolled:", rolls)
    # Add the score of player i to the list. Player 1's score is
    # stored as player_scores[0] for example.
    player_scores.append(sum(rolls))

# Compute the highest score, then check which players achieved the maximum
# score.
highest_score = max(player_scores)
highest_scoring_players = []
# We use the enumerate function so index goes from 1 to 4, with player_score
# being the corresponding score for player i.
for (index, player_score) in enumerate(player_scores, 1):
    if player_score == highest_score:
        highest_scoring_players.append(f'Player {i}')
# highest_scoring_players stores a list ['Player 1', 'Player 2'] for example
# of the winning players. We can then use join() to turn this into the string
# 'Player 1 and Player 2'.
print(' and '.join(highest_scoring_players) + ' has a higher score with:', highest_score)

I've commented to explain each individual step, and if there are any functions you're unfamiliar with, the Python documentation should be able to add further clarity. This approach has the advantage that we can easily add more players if we want by adjusting NUM_PLAYERS.

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

Welcome to code review. Great review question. I hope I can give you some suggestions to improve your Python coding style. One of the most important things you can do is to apply the DRY-principle, where DRY = Don't Repeat Yourself. Let me explain how to do this for your code.

First: use functions

Split up your code in simple tasks and create a function for each task. Separate the input functions (asks user for input), processing functions and output functions. For your game, I created the following four functions:

- throw_dice()         # more of the helper function for play_game
- play_game()          # all game-playing logic is here
- winning_players()    # logic for determining who won
- print_game_results() # outputs the results of a game

Based on the name of the functions, it should be pretty clear what each function does and how those functions work together to play the game.

One way of creating code/applications is by first splitting up your application into smaller functions (even before actually writing the code for your functions). This helps in creating the larger structure of your application instead of losing yourself into the implementation issues of smaller functions.

Use comments/docstrings

One you have identified which functions you need, try to specify them. As an example, I included Google-style Python docstrings. Give a short description, a list of input arguments and the output of the function.

Don't use magic values

Try not to use "magic numbers" in your code. Define constants (often in Python declared at the top of a module and with capitalised names) or function arguments and use them in your code. This makes it easy to override of adjust those values. Examples in the code below are DICE_12 and NUM_PLAYERS.

Use default values

The dice values or number of player can be default values (in a function call, use the = sign to declare them: def func(arg=12). arg now has a default value of 12. You can call this function without arguments (in that case, arg has the value 12, but you can also do: func(13) in which case, arg has value 13.

Use the power of Python

Python has some very powerfull concepts. In my code, I use some of them to make the code much shorter. Specifically:

  • List comprehensions: are a great way to create lists on the fly. You can look at them as for-loops on steroids. For example the code [str(n) for n in range(10)] creates a list of strings with the value 0 through 10. I use this in the winning_players() function.
  • Dict comprehensions: are similar to dict comprehensions, but for dictionaries. I use this in the play_game() function.
  • random.choices: you use randint to get an integer number, but choice picks a number from a sequence and choices can pick a number from a sequence multiple times. This is exactly what you want to do when throwing dice.

Putting all of these ingredients together, gives a much, much reduced codelength. Below is the full code with comments.

import random


# Constant defining the possible dice outcomes.
DICE_6 = list(range(1, 7))    # 6-sided dice; values 1-6
DICE_12 = list(range(1, 13))  # 12-sided dice; values 1-12

# Game parameters
NUM_PLAYERS = 4   # number of players in the game
NUM_THROWS = 5    # number of throws per player


def throw_dice(dice_sides, num_throws=1):
    """ Return a list of dice throw results.

    Args:
        dice_sides (list): List with the possible dice throw outcomes.
        num_throws (int) : The number of thows to make with the dice.

    Returns:
        list: A list of dice_sides items.
    """
    return random.choices(dice_sides, k=num_throws)


def play_game(num_players=NUM_PLAYERS, num_throws=NUM_THROWS, dice_sides=DICE_12):
    """ Play a game where each player throws num_throws times with a dice.

    Args:
        num_players (int): The number of players.
        num_throws (int) : The number of thows each player makes with the dice.
        dice_sides (list): List with the possible dice throw outcomes.

    Returns:
        dict: Dict with player number as key and the list of throws as values.
    """
    return {p: throw_dice(dice_sides, num_throws) for p in range(1, num_players + 1)}


def winning_players(game):
    """ Returns a list with the player(s) who have the maximum sum of dice throws.

    Args:
        game (dict): Dict with player number as key and the list of throws as values.

    Returns:
        list: A list with the player(s) with the highest sum of dice throws.
    """
    max_dice_sum = max(map(sum, game.values()))
    return [player for player, throws in game.items() if sum(throws) == max_dice_sum]


def print_game_results(game):
    """ Prints the results of a game throwing dice.

    Args:
        game (dict): Dict with player number as key and the list of throws as values.
    """
    for player, throws in game.items():
        print(f'Player {player} rolled {throws}. The sum of dice is: {sum(throws)}')

    players_won = winning_players(game)
    max_points = sum(game[players_won[0]])
    print(f'There is/are {len(players_won)} winning players.')
    for player in players_won:
        print(f"Player(s) {' and '.join(map(str, players_won))} won with {max_points} points.")


def main():
    game = play_game()
    print_game_results(game)


if __name__ == "__main__":
    main()

Try it online!

\$\endgroup\$
2
  • 1
    \$\begingroup\$ range(1,6) does not include 6. \$\endgroup\$ – Marc Nov 8 '20 at 15:29
  • \$\begingroup\$ @Marc: you are completely right. Corrected it. \$\endgroup\$ – agtoever Nov 8 '20 at 15: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.