david said unto the world upon 2005-12-03 20:36:
> hello again. i think my dig function is working correctly now.
> any input on how to save and restore all the rooms and descriptions?
> thanks for helping.
Hi David,
I'm catching up on mail backlog, so I'm a bit late, but I've a
suggestion or two about other aspects of your code.
> class Room:
> def __init__(self,coords):
# sniped code
> def nextdoor(self,direction):
> if direction == 'n':
> nextdoor = (self.coords[0], self.coords[1] + 1)
> return list(nextdoor)
> elif direction == 's':
> nextdoor = list((self.coords[0], self.coords[1] - 1))
> return nextdoor
> elif direction == 'e':
> nextdoor = list((self.coords[0] +1, self.coords[1]))
> return nextdoor
> elif direction == 'w':
> nextdoor = (self.coords[0] -1, self.coords[1])
> return list(nextdoor)
This looks a bit ugly to me :-)
First, I think it's a bad thing that some of your if clauses convert
the value to a list in the nextdoor assignment, and others in the
return. It is easier to grok what is going on if the like operations
are effected in the same way.
Further, why not just
return list((self.coords[0], self.coords[1] - 1))
and so forth, dispensing with the nextdoor assignment? (I get that the
name has documentation value, but I think it'd be better to do without
and put the documentation in a docstring.)
If that is changed, then all four clauses are almost identical. That
is often a sign that there is a better way. I'd suggest:
def nextdoor(self,direction):
'''direction -> list of co-ordinates for next location
(Perhaps more description here.)
'''
xy_difs = {'n':(0, 1), 's':(0, -1), 'e':(1, 0), 'w':(-1, 0)}
x_dif, y_dif = xy_difs[direction]
return list(self.coords[0] + x_dif, self.coords[1] + y_dif)
Once you get used to it, this style is often easier to understand
quickly than is a list of if tests. Perhaps best of all, this way
makes it much easier to add other directions like 'nw'.
Likewise, where you have:
> def opdir(direction):
> if direction == 'n':
> return 's'
> if direction == 's':
> return 'n'
> if direction == 'e':
> return 'w'
> if direction == 'w':
> return 'e'
I'd suggest
def opdir(direction):
return {'n':'s', 's':'n', 'e':'w', 'w':'e'}[direction]
Since that has dropped it down to a single line, it might even be
better to dispense with the function entirely. But, I'd likely leave
it, as the function provides a good place to document just what the
point of the code is.
A further advantage with what I've given over yours is what occurs if
an unexpected value is passed to opdir(). Your version will return
None (i.e. fail silently). Mine will raise an exception immediately.
>>> {'n':'s', 's':'n', 'e':'w', 'w':'e'}['bad']
Traceback (most recent call last):
File "<pyshell#6>", line 1, in -toplevel-
{'n':'s', 's':'n', 'e':'w', 'w':'e'}['bad']
KeyError: 'bad'
>>>
It often helps track down bugs if you try to arrange things so that
the problem surfaces as soon as possible.
HTH,
Brian vdB
_______________________________________________
Tutor maillist - Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor
