Anyone want to critique this program?

Chris Angelico rosuav at gmail.com
Sat Jul 2 23:02:46 EDT 2011


On Sun, Jul 3, 2011 at 12:19 PM, John Salerno <johnjsal at gmail.com> wrote:
> Just thought I'd post this in the event anyone has a few spare minutes
> and feels like tearing apart a fairly simple attempt to write a
> game. :)

Sure! You seem to comprehend the basics, which is an order of
magnitude better than some people I've tried to help...

> game_information = '***Chutes and Ladders***\nUp to four (4) players
> may play.\n'\
>                   'There are 90 spaces on the board. '\
>                   'The player to reach space 90 first wins.'

I'd do this with a triple-quoted string - it'll simply go across multiple lines:
game_information = """***Chutes and Ladders***
Up to four (4) players may play.
There are 90 spaces on the board.
The player to reach space 90 first wins."""

> c_l_spaces = {r'slid down a chute \\': [[14, 3], [20, 15], [39, 33],
> [66, 53],
>                                    [69, 58], [79, 67], [84, 71], [88,
> 36]],
>              'climbed up a ladder |=|': [[6, 17], [24, 26], [30, 44],
> [49, 62], [82, 86]]}

It strikes me that this plus populate_game_board() is a little
redundant; you could simply have the target dictionary as a literal:

spaces={14:3, 20:15, ........ 6:17, 24:26}

You can get the description "climbed up a ladder" or "slid down a
chute" by seeing if spaces[space] is more or less than space.

>            try:
>                space_type = spaces[self.space - 1][0]
>                self.space = spaces[self.space - 1][1]
>            except TypeError:
>                return (1, 'You moved to space
> {0}.'.format(self.space))
>            else:
>                return (1, 'You {0} to space {1}!'.format(space_type,
> self.space))

It strikes me as odd to use try/except/else for what's
non-exceptional. I'd either use a regular if block, or possibly
something completely different - like having a list like this:

spaces=list(range(91)) # get a list [0, 1, 2, 3... 90]

And then set the entries that have chutes/ladders, possibly from your
original dict of lists (or something like it):

for space in values:
  spaces[space[0]] = space[1]

Then to see where you end up, just go:
try:
  space=spaces[space]
except ValueError:
  print("That would take you past the end of the board!")

In this instance, except is being used for the exceptional condition,
the case where you blow past the edge of the board - rather than the
more normal situation of simply not hitting a chute/ladder.

>    players = []
>    for num in range(num_players):
>        players.append(Player(num + 1))

Generally, anything that looks like this can become a list
comprehension. It'll be faster (which won't matter here), and briefer.

players = [Player(num+1) for num in range(num_players)]

> def start_game(players):
>
> def initialize_game():
>        start_game(players)
>
> if __name__ == '__main__':
>    initialize_game()

start_game() would be more accurately called play_game(), since it
doesn't return till the game's over. And it's a little odd that
initialize_game() doesn't return till the game's over; I'd be inclined
to have initialize_game() return after initializing, and then have the
main routine subsequently call start_game() / play_game(). Just a
minor naming issue!

> 1. Are the functions I made necessary? Do they do enough or too much?

For the most part, I would say your functions are fine.

> 2. Is there a better way to implement the players than as a class?

Better way? Hard to know. There are many ways things can be done, but
the way you've chosen is as good as any. Certainly it's good enough
for the task you're doing.

> 3. Is there a better way to handle all the print calls that the
> program makes?

Again, I think it's fine. There's a general philosophy of separating
"guts" from "interface" (which would mean isolating all the print
statements from the game logic of moving pieces around), but in
something this small, the only reason to do that is for the sake of
practice. There's not much to be gained in small programs from fifty
levels of indirection.

> 4. Are my try/except blocks used appropriately?

This is the one place where I'd recommend change. Use try/except to
catch exceptional situations, not normalities. If your code is going
through the except block most of the time, there's probably something
wrong. Note I don't say there IS something wrong; it's an example of
code smell, and can be correct.

Your code has much in its favour. I've been wordy in suggestions but
that doesn't mean you have bad code! :)

Chris Angelico



More information about the Python-list mailing list