6
\$\begingroup\$

I made this game of snakes and ladders for school, and it has to have:

  • Multiple players
  • Player Names
  • Random Dice roll
  • Snakes and ladders (obviously!)
  • Subroutines

It is currently 124 lines, and is very repetitive. I need it to be at most 75 lines.

'''Take it in turns to roll the dice. Move your counter forward the number of spaces shown
on the dice. If your counter lands at the bottom of a ladder, you can move up to the top of
the ladder. If your counter lands on the head of a snake, you must slide down to the bottom
of the snake.'''
import time
import random

def Roll_dice():
    return random.randint(1,6)

def Move(Player, value, P1N, P2N, P3N, P4N):
    snake_squares = {16: 4, 33: 20, 48: 24, 62: 56, 78: 69, 94: 16}
    ladder_squares = {3: 12, 7: 23, 20: 56, 47: 53, 60: 72, 80: 94}
    Throw = Roll_dice()
    if Player == 1:
        num = value + Throw
        print(P1N, "Rolled a", Throw, "And is now on", num)
    if Player == 2:
        num = value + Throw
        print(P2N, "Rolled a", Throw, "And is now on", num)
    if Player == 3:
        num = value + Throw
    print(P3N, "Rolled a", Throw, "And is now on", num)
    if Player == 4:
        num = value + Throw
        print(P4N, "Rolled a", Throw, "And is now on", num)
    if num in snake_squares:
        print("Player got bitten by a snake and is now on square", snake_squares[num])
        num = snake_squares[num]
    elif num in ladder_squares:
        print("Player climbed a ladder and is now on square", ladder_squares[num])
        num = ladder_squares[num]
    else:
        print("",end = "")
    return num

def Setup_Players():
    players=6
    while True:
        try:
            print("How many players are in the game?")
            players = int(input())
            if players > 4 or players < 2:
                print("Must be less than 5 and greater than 1")
            else:
                return players
        except:
            print("Must be a number")



def Player_Names(NumP):
    Names = []
    for i in range(1,NumP+1):
        Names.append(input("What is the name of Player"+str(i)+"?"))
    Names.append("")
    return Names


Num_Players=Setup_Players()
P_Names = Player_Names(Num_Players)
P1N = 0
P2N = 0
P3N = 0
P4N = 0
for i in P_Names:
    if P1N == 0:
        P1N = i
        if Num_Players == 1:
            P2N, P3N, P4N = "", "", ""
            break
    elif P2N == 0:
        P2N = i
        if Num_Players == 2:
            P3N, P4N = "", ""
            break
    elif P3N == 0:
        P3N = i
        if Num_Players == 3:
            P4N = ""
            break
    elif P4N == 0:
        P4N = i
    else:
        break
print(P1N, P2N, P3N, P4N, ", Welcome To Snakes And Ladders")
input("Press Enter")
Num1 = 0
Num2 = 0
Num3 = 0
Num4 = 0
x = 0
while Num1 < 100 and Num2 < 100 and Num3 < 100 and Num4 < 100:       
    while x < Num_Players:
        x=x+1
        if x == 1:
            Num1 = Move(1, Num1, P1N, P2N, P3N, P4N)
            input("Press Enter")
            if Num1 > 99:
                print(P1N, "WINS!")
                time.sleep(3)
                exit()
        if x == 2:
            Num2 = Move(2, Num2, P1N, P2N, P3N, P4N)
            input("Press Enter")
            if Num2 > 99:
                print(P2N, "WINS!")
                time.sleep(3)
                exit()            
        if x == 3:
            Num3 = Move(3, Num3, P1N, P2N, P3N, P4N)
            input("Press Enter")
            if Num3 > 99:
                print(P3N, "WINS!")
                time.sleep(3)
                exit()
        if x == 4:
            Num4 = Move(4, Num4, P1N, P2N, P3N, P4N)
            input("Press Enter")
            if Num4 > 99:
                print(P4N, "WINS!")
                time.sleep(3)
                exit()
    x=0

I know there are no comments, but the code itself is pretty self-explanatory

As you can see, it is very 'un-pythonic'. The main problems are the main body and the Move() subroutine. They are both very repetitive. Would it be possible to condense this code to, say, less than 50 lines?

In answers, please could you just give examples of things I would change, that would still fit with the code as a whole, and how to change them. Or alternatively, a code that is not too advanced (So that I can understand it - I am Year 10 / 10th Grade in school) that still has all of the things in my code.

\$\endgroup\$
  • 1
    \$\begingroup\$ Lines of code aren't everything. You can do things in less amount of lines that are worse than if you used more lines. Please can you change your question to not be so fixated on LoC. \$\endgroup\$ – Peilonrayz Sep 26 '17 at 19:37
  • \$\begingroup\$ But that is what I want. The task is to make it so that the code is less than 70 lines long \$\endgroup\$ – AJ123 Sep 26 '17 at 21:04
5
\$\begingroup\$

First of all Lines of Code doesn't mean a thing if it is unreadable code

Dry

The don't repeat yourself principle, (which also handily removes duplicate lines and therefore greatly reduces LoC)

Your def move() and the main game which should really be in a function! are both DRY because each player has the same possible moves and that is were I can shave of the most lines.

If I change the move() function that it has as a player as argument we avoid alot of repetition. Same goes for the other if Player_Num == ??

PEP8 & Readability

There is this amazing thing called PEP, which helps all python programmers to make the code more readable, if we adhere this we can improve the following:

  1. Naming should be setup_players instead of Setup_Players for functions
  2. String formatting looks alot nicer then string concat in prints
  3. Use a if __name__ == "__main__": body
  4. Don't catch bare Exceptions.

The code, the bad and the ugly

I kinda rewrote all you had for a more readeble structure,

First of all, why 2 different functions for the stup of the game? In my opinion this would be better suited in 1 function:

def setup_game():
    players=6
    while True:
        try:
            print("How many players are in the game?")
            players = int(input())
            if players > 4 or players < 2:
                print("Must be less than 5 and greater than 1")
            else:
                break
        except ValueError:
            print("Must be a number")

    names = {}
    for i in range(1,players+1):
        while True:
            name = input("What is the name of Player {}? ".format(i))
            if not name in names:
                names[name] = 0
                break
            else:
                print('Cannot have duplicate names')
    return names

Here I return the players as a dictionary where the KEY is the players name and value the current_position, only drawback is that there cannot be any duplicate names.

Now for the juicy part!

def move_player(player, current_pos):
    snake_squares = {16: 4, 33: 20, 48: 24, 62: 56, 78: 69, 94: 16}
    ladder_squares = {3: 12, 7: 23, 20: 56, 47: 53, 60: 72, 80: 94}

    throw = roll_dice()
    next_pos = current_pos + throw
    print("{0} rolled a {1} and is now on {2}".format(player, throw, next_pos))

    if next_pos in snake_squares:
        print("Player got bitten by a snake and is now on square {}".format(snake_squares[next_pos]))
        next_pos = snake_squares[next_pos]
    elif next_pos in ladder_squares:
        print("Player climbed a ladder and is now on square {}".format(ladder_squares[next_pos]))
        next_pos = ladder_squares[next_pos]
    return next_pos

That looks alot better no more long if playerx == x, and it shows all players have to do the same.

Secondly for the main game which I rewrote in a function also no more if playrx == x because we can iterate quite nicely over a dictionary.

def game(players):
    print("{}, Welcome To Snakes And Ladders".format(" ".join(players)))
    input("Press Enter")
    while True:

        # Foreach player
        for player, current_pos in players.items():

            # Move player
            players[player] = move_player(player, current_pos)

            # Check win
            if players[player] > 100:
                return player

            # Next player
            input("Press Enter")


if __name__ == "__main__":
    players = setup_game()
    winner = game(players)
    print("Player {} won the game".format(winner))

If you HAVE to shave off some more lines, feel free to do so yourself.

\$\endgroup\$
0
\$\begingroup\$

Style

If you know the term 'un-Pythonic' you should also know the term PEP-8. Before doing anything else, pep8-ify your code.

That means:

  • Use snake_case for function and variable names.
  • Use PascalCase for class names.
  • Use ALL_CAPS for constants.

It also means:

  • Put your code in functions or classes
  • Use an if __name__ == '__main__': guard to invoke your main function.

Lists

You make use of a list one time. But in this case, list is your very best friend! You should be doing everything with a list. Not only does it model the data you have, it enables you to use list comprehensions!

def Player_Names(NumP):
    Names = []
    for i in range(1,NumP+1):
        Names.append(input("What is the name of Player"+str(i)+"?"))
    Names.append("")
    return Names

Becomes (cost: -4 lines):

def input_player_names(howmany):
    return [input("Enter player name: ").strip() for _ in range(howmany)]

Or, if you want to be forgiving (cost: 2 lines)

def read_player_name(num):
    while True:
        name = input("Enter name of player %d" % num).strip()
        if name:
            return name

def input_player_names(howmany):
    return [read_player_name(i) for i in range(howmany)]

But wait! There's more!

You can print a comma-separated list by using the str.join method:

# No!
print(P1N, P2N, P3N, P4N, ", Welcome To Snakes And Ladders")
# Yes!
players = input_player_names(num_players)
print(", ".join(players), ", welcome to Snakes and Ladders!")

You can create a list based on another list:

positions = [0 for player in players]
# or
positions = [0] * len(players)

You can use a list as an iterable, for the any and all built-in functions:

while all(pos < 100 for pos in positions):
# or 
while not any(pos >= 100 for pos in positions):

You can use the enumerate built-in function to provide a name and an index at the same time:

for i, player in enumerate(players):
    positions[i] = move(player, positions[i])

Lists are so cool, that they built them in to Python! Learn to love you some lists!

Roll_Dice

First, it should be roll_dice.

Second, it should be roll_die since "dice" is plural and "die" is singular.

Move

Your move function starts out well. The use of a dictionary for ladder and snake squares is just right: map from one value to another, and use in to detect when it occurs.

The part about checking each player variable is wrong. Just use a list, as shown above, and pass in one name and one starting location:

def move(player_name, start_pos):
    snake_squares = {16: 4, 33: 20, 48: 24, 62: 56, 78: 69, 94: 16}
    ladder_squares = {3: 12, 7: 23, 20: 56, 47: 53, 60: 72, 80: 94}

    roll = roll_die()
    print("{} rolled a '{}' and moves to {}".format(player_name, roll, start_pos + roll)

    # Now check for snakes and ladders, and print customized messages
    # using player_name instead of "Player."
\$\endgroup\$
  • \$\begingroup\$ I was able to get a print_board function in, with the boustrophedonic pattern used in the original game, in 74 lines. Woot! \$\endgroup\$ – Austin Hastings May 1 at 17:53

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.