[Tutor] my newbie program

[Magnus Lycka]

At 00:41 2002-11-26 -0600, david wrote:
>my next step is to be able to save the rooms i create.
>and current location etc.

First of all I think you need to get some kind
of event loop... Nothing happens in your program! :(

I think it's important to always keep a program
in a working state (even if it onld does a little
in the beginning) so that it can be continually tested.

>is pickle what i want to look at?

Seems like a reasonable idea.

>anyway any comments, suggestions, improvements
>much appreciated.

Some points. Avoid statements in the global scope of the
program. Just make the definitions there. So instead of
running "startroom = Room()", "map=Map()" and "me=Actor()"
where you are, put them in the end of the program like below.

This will mean that they are not declared as global variables,
and you have to adapt your classes to that (which is also an
imporovement in the long run.)

I'd make something like this.

def main()
     map = Map()
     startroom = Room(map)
     me = Actor(startroom)
     while 1:
         me.act() #This assumes a quit function in act

if __name__ == '__main__':
     main()

>class Room:
>     def __init__(self):
>         self.desc="a nondescript room"
>         self.exits=[0,0,0,0]
>
>class Map:
>     def __init__(self):
>         self.grid={(0,0):startroom}

You are making the map and room classes into pure data
containers. You put all your logic in Actor. This will
make your program very unbalanced and difficult to maintain
in the long run. Let Actor delegate what it can to Map
and Room, it will grow a lot anyway...

>class Actor:
>     def __init__(self):
>         self.location=startroom

Pass start room in as a parameter instead of using a global.

>     def act(self):
>
>         cmd = raw_input('>')
>         if len(string.split(cmd)) > 2:
>             print "too many words"

 >>> me.act()
 >dfg dfg dfg
too many words
Traceback (most recent call last):
   File "<stdin>", line 1, in ?
   File "adventure.py", line 40, in act
     if verb == 'dig':
UnboundLocalError: local variable 'verb' referenced before assignment

When you run into errors like this, a print isn't enough. You have
to prevent further execution. I suggest that you put a return statement
after the print. (Not only here I think.)

>                if dirobj == 'n':
>                     back=1
>                 if dirobj == 's':
>                     back=0
>                 if dirobj == 'e':
>                     back=3
>                 if dirobj == 'w':
>                     back=2

This block occurs two times already. Break it out to
a function.

def back(direction):
     return {'n':1, 's':0, 'e':3, 'w':2}[direction]

>me=Actor()

Actor? Hm... Have you perchance been misled by the dark Sith
Jacobson? He preaches that you should take the easy path and
divide your classes into entities that contain data and controls
(often actors) that do the work. Don't follow this path. It's
the dark side of the force. (Sorry, I saw Star Wars II on video
yesterday.) I'm still right though! ;) There might be a few cases
where it's useful to have classes more specialized to handle data
OR action, but in general it's something to avoid, and particularly
for a beginner in object-oriented programming, it's important to
try to get balanced classes. Seeing classes as either holders of
data or holder of logic as above, will degenerate the code to bad,
old structured code, and the benefits of OO is lost.


-- 
Magnus Lycka, Thinkware AB
Alvans vag 99, SE-907 50 UMEA, SWEDEN
phone: int+46 70 582 80 65, fax: int+46 70 612 80 65
http://www.thinkware.se/  mailto:magnus@thinkware.se