Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #8710 > unrolled thread

Anyone want to critique this program?

Back to article view | Back to comp.lang.python

Contents

  Anyone want to critique this program? John Salerno <johnjsal@gmail.com> - 2011-07-02 19:19 -0700
    Re: Anyone want to critique this program? Chris Angelico <rosuav@gmail.com> - 2011-07-03 13:02 +1000
      Re: Anyone want to critique this program? John Salerno <johnjsal@gmail.com> - 2011-07-02 20:41 -0700
        Re: Anyone want to critique this program? Chris Angelico <rosuav@gmail.com> - 2011-07-03 14:00 +1000
        Re: Anyone want to critique this program? "OKB (not okblacke)" <brenNOSPAMbarn@NObrenSPAMbarn.net> - 2011-07-03 18:06 +0000
          Re: Anyone want to critique this program? John Salerno <johnjsal@gmail.com> - 2011-07-03 20:25 -0700
            Re: Anyone want to critique this program? "OKB (not okblacke)" <brenNOSPAMbarn@NObrenSPAMbarn.net> - 2011-07-04 16:37 +0000
              Re: Anyone want to critique this program? Chris Angelico <rosuav@gmail.com> - 2011-07-05 03:21 +1000
                Re: Anyone want to critique this program? Ben Finney <ben+python@benfinney.id.au> - 2011-07-05 10:03 +1000
                  Re: Anyone want to critique this program? Chris Angelico <rosuav@gmail.com> - 2011-07-05 10:11 +1000
        Re: Anyone want to critique this program? Ben Finney <ben+python@benfinney.id.au> - 2011-07-04 07:37 +1000

#8710 — Anyone want to critique this program?

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. :)

I'll paste the exercise I was working on first, although I think it
was meant to be an exercise in how to use lists. I went way beyond
that, so maybe my program is overly complicated. It works, though, so
that's a plus! The main questions I had about it are at the bottom,
after the code.

----------
Snakes and Ladders can be a fun game for kids to play but you realize
when you get older that the players actually have no choices at all.
To illustrate just how little input the players have I want you to
make a computer program that allows two players to play snakes and
ladders on the board given.

	The rules of the game are simple. On each player's turn they roll one
six sided die and move ahead that many squares.  If they end their
move at the base of a ladder then they automatically climb to the top
of the ladder.  If they end their move at the end of a snake they
automatically slide down to its head. To win the game you must be the
first player to land on the last square.  If you are near the end and
your roll would cause you to go past the end square you don't move for
that turn.

	Your computerized version will look something like:
Player 1  hit enter to roll

You rolled: 3
You are at spot 3
Player 2  hit enter to roll

You rolled: 6
You climbed a ladder
You are at spot 17
although you can write this program without using lists, you should
ask yourself how you can use lists to encode where the snakes and
ladders are.
-----------------
(I changed snakes to chutes, and I didn't paste in the game board
picture. Just trust that the dictionary of lists in the beginning of
the program is accurate, and that each set of numbers represents first
the space you land on, and second the space you slide or climb to,
e.g. [30, 35] means you landed on 30, and as a result of landing on a
ladder space, you climbed up to space 35.)

-------------------------------
import random

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.'
separator = '-' * 20
spaces = [None] * 90
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]]}

class Player:

    def __init__(self, number):
        self.number = number
        self.space = 0

    def move(self, roll):
        global spaces
        if (self.space + roll) > 90:
            return (1, 'Your roll would move you off the game board.
'\
                    'You remain at space {0}.'.format(self.space))
        elif (self.space + roll) == 90:
            return (0, 'You moved to the last space. You won the
game!')
        else:
            self.space += roll
            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))

def roll_die():
    roll = random.randint(1, 6)
    print('You rolled {0}.'.format(roll))
    return roll

def populate_game_board():
    global spaces
    for key, values in c_l_spaces.items():
        for space in values:
            spaces[space[0] - 1] = [key, space[1]]

def initialize_players():
    while True:
        try:
            num_players = int(input('Enter the number of players (0 to
exit): '))
        except ValueError:
            print('You have entered an invalid response.\n')
        else:
            if num_players == 0:
                print('You have quit the game.')
                return
            elif 1 <= num_players <= 4:
                break
            elif (num_players < 0) or (num_players > 4):
                print('You have entered an invalid number of players.
\n')

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

    return players

def start_game(players):
    game = 1
    while game:
        for player in players:
            print('\n***Player {0}***'.format(player.number))
            print('You are currently on space
{0}.'.format(player.space))
            game_update = player.move(roll_die())
            print(game_update[1])
            print(separator, end='')
            if game_update[0] == 0:
                game = 0
                break
        if game:
            input('\nPress Enter for the next turn.')

def initialize_game():
    global game_information
    print(game_information)
    players = initialize_players()
    if players:
        populate_game_board()
        start_game(players)

if __name__ == '__main__':
    initialize_game()
--------------------------

My questions:

1. Are the functions I made necessary? Do they do enough or too much?
2. Is there a better way to implement the players than as a class?
3. Is there a better way to handle all the print calls that the
program makes?
4. Are my try/except blocks used appropriately?

Any other comments would be appreciated too!
Thanks!

[toc] | [next] | [standalone]

#8713

On Sun, Jul 3, 2011 at 12:19 PM, John Salerno <johnjsal@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

[toc] | [prev] | [next] | [standalone]

#8717

On Jul 2, 10:02 pm, Chris Angelico <ros...@gmail.com> wrote:

> > 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."""

Yeah, I considered that, but I just hate the way it looks when the
line wraps around to the left margin. I wanted to line it all up under
the opening quotation mark. The wrapping may not be as much of an
issue when assigning a variable like this, but I especially don't like
triple-quoted strings that wrap around inside function definitions.
That seems to completely throw off the indentation. Do people still
use triple-quotes in that situation?

> > 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.

Hmm, interesting. I'm not quite sure how I'd implement that yet, but
the "14:3" structure seems cleaner (although I admit at first it
looked weird, because I'm used to seeing strings as dictionary
keywords!).

> >            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.

Originally I didn't have the try/except at all, I had nested if
statements that seemed to be getting out of control. Then I realized
if I just attempted to index the list and handle the exception, rather
than check first if indexing the list was allowed, the code came out
cleaner looking. But I agree with you, my "exception" is actually the
more frequent occurrence, so I'm going to change that.

> >    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)]

Now this is the kind of tip I love. Something like list comprehensions
just didn't even occur to me, so I'm definitely going to change that.

> > 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!

Minor or not, it makes sense. I'm picky about things like that too, so
now that you've pointed it out, I'm compelled to change the names so
they make sense! :)

> > 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.

Yeah, I don't need to know 10 different ways to do things. Mainly I
asked this question because the original exercise seemed to focus on
using lists to write the game, and I just couldn't think of an
efficient way to use a list to store the player attributes like what
space they were on. It just seemed a natural candidate for a class
attribute.

> > 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.

Heh, then you should have seen the original version I wrote! All the
print calls were *inside* the "move" method, but it felt strange to
have the move method do the printing directly, so I changed it to a
return statement and handled the printing elsewhere. Although I still
wonder if I could abstract the print calls even further. The presence
of all that text just seems like it begs for a clean way to handle it.

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

Thanks a lot for the tips! I'm going to go back over the whole thing
and rework some of it. And I'm only doing this for the sake of
learning, so even the small tips help me to think more like a
programmer. :)

[toc] | [prev] | [next] | [standalone]

#8720

On Sun, Jul 3, 2011 at 1:41 PM, John Salerno <johnjsal@gmail.com> wrote:
> Yeah, I considered that, but I just hate the way it looks when the
> line wraps around to the left margin. I wanted to line it all up under
> the opening quotation mark. The wrapping may not be as much of an
> issue when assigning a variable like this, but I especially don't like
> triple-quoted strings that wrap around inside function definitions.
> That seems to completely throw off the indentation. Do people still
> use triple-quotes in that situation?

Jury's out on that one. You can either back-tab it to the left (looks
ugly), or indent it and then clean it up in code (IS ugly). Up to you
to figure out what's best for your code, and if you want to indent it,
your current way is quite possibly the cleanest.

> Minor or not, it makes sense. I'm picky about things like that too, so
> now that you've pointed it out, I'm compelled to change the names so
> they make sense! :)

:) Names tend to stay the same when the functions they're attached to
grow and shift. Sometimes you end up with these great warts in huge
production code... it's sometimes just too much work to change things.

> Thanks a lot for the tips! I'm going to go back over the whole thing
> and rework some of it. And I'm only doing this for the sake of
> learning, so even the small tips help me to think more like a
> programmer. :)

Learning's good! And Python's an excellent language for the purpose.
Code quickly, invoke quickly (no compilation stage), and see the
results of the work.

ChrisA

[toc] | [prev] | [next] | [standalone]

#8734

John Salerno wrote:

> On Jul 2, 10:02 pm, Chris Angelico <ros...@gmail.com> wrote:
>> 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."""
> 
> Yeah, I considered that, but I just hate the way it looks when the
> line wraps around to the left margin. I wanted to line it all up
> under the opening quotation mark. The wrapping may not be as much
> of an issue when assigning a variable like this, but I especially
> don't like triple-quoted strings that wrap around inside function
> definitions. That seems to completely throw off the indentation. Do
> people still use triple-quotes in that situation?

    	I do, because I use an editor that intelligently indents wrapped 
text to the same indent level as the beginning of the line, instead of 
wrapping it all the way back to the margin.

-- 
--OKB (not okblacke)
Brendan Barnwell
"Do not follow where the path may lead.  Go, instead, where there is
no path, and leave a trail."
	--author unknown

[toc] | [prev] | [next] | [standalone]

#8753

On Jul 3, 1:06 pm, "OKB (not okblacke)"
<brenNOSPAMb...@NObrenSPAMbarn.net> wrote:

> > Yeah, I considered that, but I just hate the way it looks when the
> > line wraps around to the left margin. I wanted to line it all up
> > under the opening quotation mark. The wrapping may not be as much
> > of an issue when assigning a variable like this, but I especially
> > don't like triple-quoted strings that wrap around inside function
> > definitions. That seems to completely throw off the indentation. Do
> > people still use triple-quotes in that situation?
>
>         I do, because I use an editor that intelligently indents wrapped
> text to the same indent level as the beginning of the line, instead of
> wrapping it all the way back to the margin.

But isn't wrapped text something different than text that is purposely
split across multiple lines with a newline character? That's usually
the case when I need to split up a long string.

[toc] | [prev] | [next] | [standalone]

#8776

John Salerno wrote:

> On Jul 3, 1:06 pm, "OKB (not okblacke)"
> <brenNOSPAMb...@NObrenSPAMbarn.net> wrote:
> 
>> > Yeah, I considered that, but I just hate the way it looks when the
>> > line wraps around to the left margin. I wanted to line it all up
>> > under the opening quotation mark. The wrapping may not be as much
>> > of an issue when assigning a variable like this, but I especially
>> > don't like triple-quoted strings that wrap around inside function
>> > definitions. That seems to completely throw off the indentation. Do
>> > people still use triple-quotes in that situation? 
>>
>>         I do, because I use an editor that intelligently indents 
>> wrapped text to the same indent level as the beginning of the line,
>> instead of wrapping it all the way back to the margin. 
> 
> But isn't wrapped text something different than text that is purposely
> split across multiple lines with a newline character? That's usually
> the case when I need to split up a long string.

    	Well, what I'm saying is I use an editor that lets me make the 
lines as long as I want, and it still wraps them right, so I never 
explicitly hit enter to break a line except at the end of a string (or 
paragraph).

-- 
--OKB (not okblacke)
Brendan Barnwell
"Do not follow where the path may lead.  Go, instead, where there is
no path, and leave a trail."
	--author unknown

[toc] | [prev] | [next] | [standalone]

#8779

On Tue, Jul 5, 2011 at 2:37 AM, OKB (not okblacke)
<brenNOSPAMbarn@nobrenspambarn.net> wrote:
>        Well, what I'm saying is I use an editor that lets me make the
> lines as long as I want, and it still wraps them right, so I never
> explicitly hit enter to break a line except at the end of a string (or
> paragraph).

In this instance, I believe the OP was paragraphing his text. Is there
a convenient way to do that in a triple-quoted string?

My personal inclination would be to simply back-tab it. It looks ugly,
but at least it works, and doesn't require a run-time re-parse. The
run-time translation is good for docstrings, though (which are
syntactically the same thing as this situation).

ChrisA

[toc] | [prev] | [next] | [standalone]

#8804

Chris Angelico <rosuav@gmail.com> writes:

> In this instance, I believe the OP was paragraphing his text.

What is “paragraphing”?

> Is there a convenient way to do that in a triple-quoted string?

Did I answer this earlier in the thread (also at
<URL:http://stackoverflow.com/questions/2504411/proper-indentation-for-python-multiline-strings/2504454#2504454>)?

> My personal inclination would be to simply back-tab it. It looks ugly,
> but at least it works, and doesn't require a run-time re-parse.

Readability counts. Why is “doesn't require a run-time re-parse” more
valuable than readability?

-- 
 \           “Because of the impropriety of entertaining guests of the |
  `\    opposite sex in the bedroom, it is suggested that the lobby be |
_o__)                           used for this purpose.” —hotel, Zurich |
Ben Finney

[toc] | [prev] | [next] | [standalone]

#8806

On Tue, Jul 5, 2011 at 10:03 AM, Ben Finney <ben+python@benfinney.id.au> wrote:
> Chris Angelico <rosuav@gmail.com> writes:
>
>> In this instance, I believe the OP was paragraphing his text.
>
> What is “paragraphing”?

If you look at the original code, you'll see embedded newlines used to
create multiple paragraphs. Hence, paragraphing as opposed to simply
wrapping in order to keep his source lines <80 chars.

>> My personal inclination would be to simply back-tab it. It looks ugly,
>> but at least it works, and doesn't require a run-time re-parse.
>
> Readability counts. Why is “doesn't require a run-time re-parse” more
> valuable than readability?

Readability definitely counts. But having a string literal require an
extra call to make it what you want also costs readability. It's like
pushing your literals off to an i18n file - what you gain in
flexibility, you lose in simplicity.

ChrisA

[toc] | [prev] | [next] | [standalone]

#8738

John Salerno <johnjsal@gmail.com> writes:

> On Jul 2, 10:02 pm, Chris Angelico <ros...@gmail.com> wrote:
>
> > > 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."""
>
> Yeah, I considered that, but I just hate the way it looks when the
> line wraps around to the left margin.

    import textwrap

    game_information = textwrap.dedent("""\
        ***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.
        """)

-- 
 \      “Nothing is more sacred than the facts.” —Sam Harris, _The End |
  `\                                                   of Faith_, 2004 |
_o__)                                                                  |
Ben Finney

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web