65
\$\begingroup\$

I was advised by a Reddit user to get my code reviewed on this site.

The complete code is on GitHub.

# Monopoly Simulator
# http://img.thesun.co.uk/aidemitlum/archive/01771/Monopoly2_1771742a.jpg

from random import randint

piece = 0
jail = 0
iteration = 0

brown1 = 0
brown2 = 0

light_blue1 = 0
light_blue2 = 0
light_blue3 = 0

pink1 = 0
pink2 = 0
pink3 = 0

orange1 = 0
orange2 = 0
orange3 = 0

red1 = 0
red2 = 0
red3 = 0

yellow1 = 0
yellow2 = 0
yellow3 = 0

green1 = 0
green2 = 0
green3 = 0

dark_blue1 = 0
dark_blue2 = 0

num = input("How many rolls do you want to simulate? ")

for h in range(0, num):
    piece += randint(2, 12)

    if piece > 40:
        piece -= 40
        iteration += 1
        #print("Around the board %d times so far" %(iteration)) ### Optional

    #print(piece) ### Optional

    #Jail
    if piece == 30:
        piece = 10
        jail += 1
        #print("JAIL") ### Optional

    #Brown
    if piece == 1:
        brown1 += 1
    if piece == 3:
        brown2 += 1

    #Light Blue
    if piece == 6:
        light_blue1 += 1
    if piece == 8:
        light_blue2 += 1
    if piece == 9:
        light_blue3 += 1

    #Pink
    if piece == 11:
        pink1 += 1
    if piece == 13:
        pink2 += 1
    if piece == 14:
        pink3 += 1

    #Orange
    if piece == 16:
        orange1 += 1
    if piece == 18:
        orange2 += 1
    if piece == 19:
        orange3 += 1

    #Red
    if piece == 21:
        red1 += 1
    if piece == 23:
        red2 += 1
    if piece == 24:
        red3 += 1

    #Yellow
    if piece == 26:
        yellow1 += 1
    if piece == 27:
        yellow2 += 1
    if piece == 29:
        yellow3 += 1

    #Green
    if piece == 31:
        green1 += 1
    if piece == 32:
        green2 += 1
    if piece == 34:
        green3 += 1

    #Dark Blue
    if piece == 37:
        dark_blue1 += 1
    if piece == 39:
        dark_blue2 += 1

brown = brown1 + brown2
light_blue = light_blue1 + light_blue2 + light_blue3
pink = pink1 + pink2 + pink3
orange = orange1 + orange2 + orange3
red = red1 + red2 + red3
yellow = yellow1 + yellow2 + yellow3
green = green1 + green2 + green3
dark_blue = dark_blue1 + dark_blue2

#Prints all the Statistics

print("\n\n")
print("Brown = %d" %(brown))
print("Light Blue = %d" %(light_blue))
print("Pink = %d" %(pink))
print("Orange = %d" %(orange))
print("Red = %d" %(red))
print("Yellow = %d" %(yellow))
print("Green = %d" %(green))
print("Dark Blue = %d" %(dark_blue))
print("\n")

print("Brown 1 = %d" %(brown1))
print("Brown 2 = %d" %(brown2))
print("\n")
print("Light Blue 1 = %d" %(light_blue1))
print("Light Blue 2 = %d" %(light_blue2))
print("Light Blue 3 = %d" %(light_blue3))
print("\n")
print("Pink 1 = %d" %(pink1))
print("Pink 2 = %d" %(pink2))
print("Pink 3 = %d" %(pink3))
print("\n")
print("Orange 1 = %d" %(orange1))
print("Orange 2 = %d" %(orange2))
print("Orange 3 = %d" %(orange3))
print("\n")
print("Red 1 = %d" %(red1))
print("Red 2 = %d" %(red2))
print("Red 3 = %d" %(red3))
print("\n")
print("Yellow 1 = %d" %(yellow1))
print("Yellow 2 = %d" %(yellow2))
print("Yellow 3 = %d" %(yellow3))
print("\n")
print("Green 1 = %d" %(green1))
print("Green 2 = %d" %(green2))
print("Green 3 = %d" %(green3))
print("\n")
print("Dark Blue 1 = %d" %(dark_blue1))
print("Dark Blue 2 = %d" %(dark_blue2))
print("\n")
print("You've been jailed %d times" %(jail))



#The Board

#Calculating highest number of digits (for board formatting)

places = [brown1, brown2, light_blue1, light_blue2, light_blue3, pink1, pink2, pink3,
        orange1, orange2, orange3, red1, red2, red3, yellow1, yellow2, yellow3,
        green1, green2, green3, dark_blue1, dark_blue2]

digit = 0
temp = 0

for place in places:
    while place / 10 >= 1:
        place /= 10
        temp += 1
    temp += 1
    if temp > digit:
        digit = temp
    temp = 0

#Creating Blanks & Spaces

blank = "-"
space = " "

for i in range(0, digit - 1):
    blank += "-"
    space += " "

#Formatting all the places, so that they have "temp" digits

formatted = []

placelen = 0

for place in places:
    holder = place
    form = 0

    while holder / 10 >= 1:
        holder /= 10
        placelen += 1
    placelen += 1

    if placelen != digit:
        form = format(place, "0%d" %(digit))
    else:
        form = str(place)

    placelen = 0

    formatted.append(form)


brown1 = formatted[0]
brown2 = formatted[1]

light_blue1 = formatted[2]
light_blue2 = formatted[3]
light_blue3 = formatted[4]

pink1 = formatted[5]
pink2 = formatted[6]
pink3 = formatted[7]

orange1 = formatted[8]
orange2 = formatted[9]
orange3 = formatted[10]

red1 = formatted[11]
red2 = formatted[12]
red3 = formatted[13]

yellow1 = formatted[14]
yellow2 = formatted[15]
yellow3 = formatted[16]

green1 = formatted[17]
green2 = formatted[18]
green3 = formatted[19]

dark_blue1 = formatted[20]
dark_blue2 = formatted[21]

#Making the Board

board = [
    [blank, red1, blank, red2, red3, blank, yellow1, yellow2, blank, yellow3, blank],
    [orange1, space, space, space, space, space, space, space, space, space, green1],
    [orange2, space, space, space, space, space, space, space, space, space, green2],
    [blank, space, space, space, space, space, space, space, space, space, blank],
    [orange3, space, space, space, space, space, space, space, space, space, green3],
    [blank, space, space, space, space, space, space, space, space, space, blank],
    [pink3, space, space, space, space, space, space, space, space, space, blank],
    [pink2, space, space, space, space, space, space, space, space, space, dark_blue1],
    [blank, space, space, space, space, space, space, space, space, space, blank],
    [pink1, space, space, space, space, space, space, space, space, space, dark_blue2],
    [blank, light_blue1, light_blue2, blank, light_blue3, blank, blank, brown2, blank, brown1, "GO"]
    ]

#Drawing the Board

print("\n")

print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[0][0], 
                                                                                board[0][1],
                                                                                board[0][2],
                                                                                board[0][3],
                                                                                board[0][4],
                                                                                board[0][5],
                                                                                board[0][6],
                                                                                board[0][7],
                                                                                board[0][8],
                                                                                board[0][9],
                                                                                board[0][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[1][0], 
                                                                                board[1][1],
                                                                                board[1][2],
                                                                                board[1][3],
                                                                                board[1][4],
                                                                                board[1][5],
                                                                                board[1][6],
                                                                                board[1][7],
                                                                                board[1][8],
                                                                                board[1][9],
                                                                                board[1][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[2][0], 
                                                                                board[2][1],
                                                                                board[2][2],
                                                                                board[2][3],
                                                                                board[2][4],
                                                                                board[2][5],
                                                                                board[2][6],
                                                                                board[2][7],
                                                                                board[2][8],
                                                                                board[2][9],
                                                                                board[2][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[3][0], 
                                                                                board[3][1],
                                                                                board[3][2],
                                                                                board[3][3],
                                                                                board[3][4],
                                                                                board[3][5],
                                                                                board[3][6],
                                                                                board[3][7],
                                                                                board[3][8],
                                                                                board[3][9],
                                                                                board[3][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[4][0], 
                                                                                board[4][1],
                                                                                board[4][2],
                                                                                board[4][3],
                                                                                board[4][4],
                                                                                board[4][5],
                                                                                board[4][6],
                                                                                board[4][7],
                                                                                board[4][8],
                                                                                board[4][9],
                                                                                board[4][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[5][0], 
                                                                                board[5][1],
                                                                                board[5][2],
                                                                                board[5][3],
                                                                                board[5][4],
                                                                                board[5][5],
                                                                                board[5][6],
                                                                                board[5][7],
                                                                                board[5][8],
                                                                                board[5][9],
                                                                                board[5][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[6][0], 
                                                                                board[6][1],
                                                                                board[6][2],
                                                                                board[6][3],
                                                                                board[6][4],
                                                                                board[6][5],
                                                                                board[6][6],
                                                                                board[6][7],
                                                                                board[6][8],
                                                                                board[6][9],
                                                                                board[6][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[7][0], 
                                                                                board[7][1],
                                                                                board[7][2],
                                                                                board[7][3],
                                                                                board[7][4],
                                                                                board[7][5],
                                                                                board[7][6],
                                                                                board[7][7],
                                                                                board[7][8],
                                                                                board[7][9],
                                                                                board[7][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[8][0], 
                                                                                board[8][1],
                                                                                board[8][2],
                                                                                board[8][3],
                                                                                board[8][4],
                                                                                board[8][5],
                                                                                board[8][6],
                                                                                board[8][7],
                                                                                board[8][8],
                                                                                board[8][9],
                                                                                board[8][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[9][0], 
                                                                                board[9][1],
                                                                                board[9][2],
                                                                                board[9][3],
                                                                                board[9][4],
                                                                                board[9][5],
                                                                                board[9][6],
                                                                                board[9][7],
                                                                                board[9][8],
                                                                                board[9][9],
                                                                                board[9][10]
                                                                                ))
print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(board[10][0], 
                                                                                board[10][1],
                                                                                board[10][2],
                                                                                board[10][3],
                                                                                board[10][4],
                                                                                board[10][5],
                                                                                board[10][6],
                                                                                board[10][7],
                                                                                board[10][8],
                                                                                board[10][9],
                                                                                board[10][10]
                                                                                ))

I've only been programming for about a month now, so I know that there are many ways to improve and optimize my code. Just looking through it, I seem to be repeating some code so abstracting them using functions could be a possibility.

I should mention that I am aware of the mistake of simply doing randint(2, 12). On my Github page, I have changed it so that I do randint(1, 6) twice and add them together. This is important because it changes the probability.

I appreciate any help and suggestions that you can offer!

\$\endgroup\$
  • 24
    \$\begingroup\$ Note that committing a Monopoly Simulator v2.py defeats the purpose of the version control system, which is to track the history of your changes to each file. \$\endgroup\$ – 200_success May 31 '16 at 16:51
  • \$\begingroup\$ And @200_success , could you please explain the version control system on Github? I'm new to the whole programming scene so I don't fully know how Github works. I appreciate your help! \$\endgroup\$ – ohjuny May 31 '16 at 17:38
  • 11
    \$\begingroup\$ The quickie Git tutorial: (1) Edit your files. (2) git add file1.py file2.py (3) git commit (4) git push (5) GOTO 1. Then Git will track the modifications for you within each file, and you can visualize those changes when browsing GitHub. Unfortunately, a full Git tutorial is too large to fit in a comment, as there is enough material for a book. \$\endgroup\$ – 200_success May 31 '16 at 17:43
  • \$\begingroup\$ Does Python 2 allow you do coerce a string to an int? Line 42 (for h in range) is giving me an error since it expects 2 ints, and it's getting an int and a string. \$\endgroup\$ – Carcigenicate Jun 2 '16 at 14:08
  • \$\begingroup\$ @Carcigenicate In that for loop, the "num" is not a string - it is a variable which holds an int. So there should be no error regardless of which Python you are running. Also, I think that Python 2 does allow coercion. If coercing an string to an int, you do int("string"). \$\endgroup\$ – ohjuny Jun 2 '16 at 16:54
68
\$\begingroup\$

You really need to remove some of your global variables. As an example I'd change all your browns to use a list instead.

brown = [0, 0]

As you do a lot of logic over brown, cyan, pink, etc, I'd make a dictionary. Dictionary's are like lists, they have a few more features that I'd use and are basically lists but can have 'any' key.

And so I'd use:

places = {
    'Brown': [0, 0],
    'Cyan': [0, 0, 0],
    'Pink': [0, 0, 0],
    'Orange': [0, 0, 0],
    'Red': [0, 0, 0],
    'Yellow': [0, 0, 0],
    'Green': [0, 0, 0],
    'Blue': [0, 0, 0]
}

A lot of your logic is based around where these places are. To reduce duplicate information I'd make another dictionary, telling us which piece is there. And so I'd use:

board = {
    1: ('Brown', 0),
    3: ('Brown', 1),
    6: ('Cyan', 0),
    8: ('Cyan', 1),
    9: ('Cyan', 2),
    11: ('Pink', 0),
    13: ('Pink', 1),
    14: ('Pink', 2),
    16: ('Orange', 0),
    18: ('Orange', 1),
    19: ('Orange', 2),
    21: ('Red', 0),
    23: ('Red', 1),
    24: ('Red', 2),
    26: ('Yellow', 0),
    27: ('Yellow', 1),
    29: ('Yellow', 2),
    31: ('Green', 0),
    32: ('Green', 1),
    34: ('Green', 2),
    37: ('Blue', 0),
    39: ('Blue', 1),
}

As you should see this is easier to use than a list, as we don't have to provide 0. I would then go on to change almost all your logic to use these. It will significantly reduce the amount of lines in your code and will make your code easier to read and understand.

First things first I'd change your first loop to use this. I'd not change iteration and jail as they don't fit into the objects we created above. However I'd change all your other ifs to use dictionary.get. With a bit of tuple unpacking you can simplify them all to:

house_set, place = board.get(piece, (None, None))
if house_set is not None:
    places[house_set][place] += 1

I'd then go on to change how you calculate and display the totals. Using a dictionary comprehension and sum. You want the sum of the 'house_set' and to know the 'place'. And to get both you need to go through dict.items().

totals = {place: sum(house_set) for place, house_set in places.items()}

To then display this you can use a for loop over totals.items(). Where you display the place and the amount.

for place, amount in totals.items():
    print('{} = {}'.format(place, amount))

After this you display all the numbers, from brown 1 to blue 2. I'd use the same as above but using places rather than totals. However this returns a list that we'll have to loop through. I'd loop through this and display them as we did above but you need to know it's 'brown 1' rather than 'brown'. And so I'd use enumerate with the optional argument to simplify the logic to getting these numbers. I'd then use:

for place, house_set in places.items():
    for i, amount in enumerate(house_set, 1):
        print('{} {} = {}'.format(place, i, amount))

Finally you print the amount of times you've been jailed before you print a nice looking board. This is roughly the same as before, but as I use str.format rather than % I'd change it to keep consistent.

print("You've been jailed {} times".format(jail))

I'd drastically change how you display the board, I'm not saying this is the best way. It's just simpler than typing space and blank a lot. It will also show that the board is actually just a line.

Currently you use a while loop and divide by ten a lot to find the amount of digits the number has. This is alright but not as clear as changing it to a string and finding it's length. You also find the largest number to get this length from. This is by manually writing places instead I'd use a list comprehension. As we will go through (my) places again but we don't need the name you can use dict.values() to get the values. And so would result in:

digit = len(str(max(amount for house_set in places.values() for amount in house_set)))

After this you make blank and space this size manually. Instead you can use " " * digit. This will allow you to duplicate the string that many times. And so simplifies the code:

blank = "-" * digit
space = " " * digit

After this you make a formatted list. I'd instead format places. However to be able to keep the original I'd use a dictionary comprehension and a list comprehension. This will be like the for loops we used earlier to display the numbers. I'd also use str.format the format you are using is simple in this mini-language. You want the number to be proceeded by zeros, and so you'd use something like {:0>digit}. However you want to pass the digit. To simplify the logic you can use {{:0>{}}}.format(digit).format(data). Where data is the data we will be formatting, as you did before. And so I'd use:

place_format = '{{:0>{}}}'.format(digit)

formatted = {
    place: [place_format.format(amount) for amount in house_set]
    for place, house_set in places.items()
}

Finally I'll use three more steps where you used two. This is to reduce duplicated logic, but adds some code that is harder to read. As we constructed the board's positions earlier in board we know where to place things in the boards line. We also know that this line is around the outer of the square board, and you chose to start the origin at the bottom right.

Before this get's too complicated I'll explain how I'd make the board's line. You know there are 40 places on the board. Of these 40 you have said all the places where houses/streets are. This allows you to use the dictionary we constructed before with board.get again. If there is no piece in that dictionary than we know that it should be blank. Finally no matter what, the first item on the board is 'go'. This allows you to do:

board_line = []
for index in range(40):
    place = board.get(index, None)
    if place is None:
        value = blank
    else:
        value = places[place[0]][place[1]]
    board_line.append(value)
board_line[0] = 'Go'

After this we can translate that list to a board. This is by getting the x and y position of the board from the 40 index's. As this is mostly maths I'll just dump the code. But I initialize the entire board as space and overwrite the outer circle with this code to the line above. This is with:

board = [[space] * 11 for _ in range(11)]
for index in range(40):
    x, y = 0, 0
    if index < 11:
        x = 10 - index
        y = 10
    elif index < 21:
        x = 0
        y = 10 - (index - 10)
    elif index < 31:
        x = index - 20
    else:
        x = 10
        y = index - 30
    board[y][x] = board_line[index]

Finally, I'd simplify the prints. You should loop through each line of the board and print it. As you add some space either side of the board I'd use str.format again to wrap the result of str.join. And as you want to join each square of the board with | you should use ' | '.join(line). This can result in:

for line in board:
    print('  {}  '.format('  |  '.join(str(p) for p in line)))

Here's all the code:

# Monopoly Simulator
# http://img.thesun.co.uk/aidemitlum/archive/01771/Monopoly2_1771742a.jpg

from random import randint

board = {
    1: ('Brown', 0),
    3: ('Brown', 1),
    6: ('Cyan', 0),
    8: ('Cyan', 1),
    9: ('Cyan', 2),
    11: ('Pink', 0),
    13: ('Pink', 1),
    14: ('Pink', 2),
    16: ('Orange', 0),
    18: ('Orange', 1),
    19: ('Orange', 2),
    21: ('Red', 0),
    23: ('Red', 1),
    24: ('Red', 2),
    26: ('Yellow', 0),
    27: ('Yellow', 1),
    29: ('Yellow', 2),
    31: ('Green', 0),
    32: ('Green', 1),
    34: ('Green', 2),
    37: ('Blue', 0),
    39: ('Blue', 1),
}

places = {
    'Brown': [0, 0],
    'Cyan': [0, 0, 0],
    'Pink': [0, 0, 0],
    'Orange': [0, 0, 0],
    'Red': [0, 0, 0],
    'Yellow': [0, 0, 0],
    'Green': [0, 0, 0],
    'Blue': [0, 0, 0]
}

piece = 0
jail = 0
iteration = 0

num = input("How many rolls do you want to simulate? ")
for h in range(num):
    piece += randint(1, 6) + randint(1, 6)

    if piece > 40:
        piece -= 40
        iteration += 1

    if piece == 30:
        piece = 10
        jail += 1

    house_set, place = board.get(piece, (None, None))
    if house_set is not None:
        places[house_set][place] += 1

totals = {place: sum(house_set) for place, house_set in places.items()}

for place, amount in totals.items():
    print('{} = {}'.format(place, amount))

for place, house_set in places.items():
    for i, amount in enumerate(house_set, 1):
        print('{} {} = {}'.format(place, i, amount))

print("You've been jailed %d times" %(jail))

digit = len(str(max(amount for house_set in places.values() for amount in house_set)))
blank = "-" * digit
space = " " * digit
place_format = '{{:0>{}}}'.format(digit)

formatted = {
    place: [place_format.format(amount) for amount in house_set]
    for place, house_set in places.items()
}

board_line = []
for index in range(40):
    place = board.get(index, None)
    if place is None:
        value = blank
    else:
        value = places[place[0]][place[1]]
    board_line.append(value)
board_line[0] = 'Go'

board = [[space] * 11 for _ in range(11)]
for index in range(40):
    x, y = 0, 0
    if index < 11:
        x = 10 - index
        y = 10
    elif index < 21:
        x = 0
        y = 10 - (index - 10)
    elif index < 31:
        x = index - 20
    else:
        x = 10
        y = index - 30
    board[y][x] = board_line[index]

for line in board:
    print('  {}  '.format('  |  '.join(str(p) for p in line)))
\$\endgroup\$
  • 18
    \$\begingroup\$ Wow, I really appreciate the time and effort you put in to help me! This was actually really helpful, even though I don't fully understand everything just yet (but I can go look it up). Clearly, I have a long way to go in the world of programming, but I'm looking forward to the journey! \$\endgroup\$ – ohjuny May 31 '16 at 17:36
  • 4
    \$\begingroup\$ @ohjuny no problem at all! Happy it helped! If you learn the main things I mention, lists, dictionarys (dicts) and comprehensions you'll have a much more enjoyable journey, :) \$\endgroup\$ – Peilonrayz May 31 '16 at 17:50
  • 3
    \$\begingroup\$ Actually there are 41 spaces on the board. "In Jail" and "Just Visiting" are distinct entities. Counting as you have, with Go as 0, I'd treat "Just Visiting" as 10, and make "In Jail" be a separate entity not on the 0..39 track at all. When a player leaves Jail, just move them to 10:JV and apply the out roll from there. \$\endgroup\$ – Monty Harder Jun 1 '16 at 16:55
  • \$\begingroup\$ @MontyHarder That's a fair point. I actually see the Jail as a single space, with two sub-spaces. And is how I'd implement it. But to each their own, :) But I agree I wouldn't add a new space in the 0..39 for it. \$\endgroup\$ – Peilonrayz Jun 1 '16 at 17:33
  • 1
    \$\begingroup\$ @JoeWallis It sounds like your "sub-spaces" are my "spaces". In your code above, you show Jail as 10, but have a separate Jail flag. I'd implement the Jail variable to mean "number of free roll-out attempts remaining", set initially to 3 when first jailed, and decremented on each roll. If the player rolls doubles, set Jail back to 0 and move out. If Jail reaches 0 on roll-out attempts, subtract $50 from player's money; they're now in Just Visiting by virtue of Jail=0 again. \$\endgroup\$ – Monty Harder Jun 1 '16 at 18:40
26
\$\begingroup\$

You have a lot of verbose repetition, you should learn to simplify things with loops and functions.

First, you're printing the board with each line getting a print statement, but you could just use a for loop:

for line in board:
    print("  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  |  %s  " %(line[0], 
                                                                                           line[1],
                                                                                           line[2],

But even better than that, you can use str.join to print a list in one go. The way you use it is to call .join on the string you want to use as a separator. In your case, you can use ' | '. So you want to join each line of the board with the string |.

for line in board:
    print("  |  ".join(line))

Lastly, I'd put this in a function so it's easier to call and recall. Plus it will look neater.

def draw(board):
    """Print out the whole board"""

    print("\n")
    for line in board:
        print("  |  ".join(line))
\$\endgroup\$
  • 3
    \$\begingroup\$ @ohjuny Loop everything that can be looped. If you're almost identically duplicating lines, try to change it to a loop. That single lesson will save you tons of unneeded work. \$\endgroup\$ – Carcigenicate Jun 2 '16 at 17:34
  • \$\begingroup\$ @Carcigenicate Yes, that is one thing I will take away from this experience. I've actually tried making another Monopoly board, with a bit more functionality, and I've tried incorporating a dictionary and more loops. You can check it out here and tell me what you think! :D \$\endgroup\$ – ohjuny Jun 3 '16 at 12:04
  • 1
    \$\begingroup\$ @ohjuny I personally don't have time to look at that right now, but if you're finished making changes you can post a new question on the site with your updated code and link back to here explaining that it's a version 2 of your code. \$\endgroup\$ – SuperBiasedMan Jun 3 '16 at 15:18
4
\$\begingroup\$

On every Monopoly board I have ever played, the first two properties have a purple header, not brown, so I was confused to see Brown1 and Brown2.

It doesn't really matter what the colors are, but when you are writing code to simulate something, it is important that your variables and values match the expectations of people who are familiar with the object being simulated.

If there are different boards with different property colors, then it would be better to not use the colors as variables or values, instead using some other accurate identifier scheme.

\$\endgroup\$
  • 3
    \$\begingroup\$ You've only played on an older board then. monopoly.wikia.com/wiki/Brown_Color_Group_Properties \$\endgroup\$ – duckbrain Jun 1 '16 at 17:39
  • \$\begingroup\$ Not going to share my age :) There have been what seems an infinite number of customized Monopoly versions, the colors were bound to change at some point. \$\endgroup\$ – cdkMoose Jun 1 '16 at 18:01
  • 4
    \$\begingroup\$ @Duckbrain, Colors changed in 2008 and the game has been around since the 1930s, many developers are of an age where their childhood versions of the game pre-date the change. \$\endgroup\$ – cdkMoose Jun 1 '16 at 18:40
  • 6
    \$\begingroup\$ @cdkMoose Are you sure they changed it in 2008 to brown, apparently the UK one has been brown since 1935, I've also never seen them as purple in the UK. \$\endgroup\$ – Peilonrayz Jun 1 '16 at 19:21
  • 5
    \$\begingroup\$ IMO that makes a compelling reason to deem the "color" abstraction a bad one, and use a numbered "group" abstraction instead, where the color is just an attribute, not the name being referred to in-code. \$\endgroup\$ – Mathieu Guindon Jun 2 '16 at 14:55

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