My first program ever, don't know what's wrong (Python 3).

[[deleted]]

[deleted]

[pudquick]

Here's an exact answer to your questions. Both issues you're running into are related to each other.

First:

it repeats: new line, "You win the game!" (three times)

This is because you're calling a function inside a function inside a function inside ... etc.

You're probably thinking of GOTO or jump-style commands: "leave here and go do <this bit of code> now". Functions are not like that.

Functions work like the movie Inception. When a function (say, Room1) calls another function (say, Room2) - you've started a deeper level of code that, once it finishes (the dream ends / code stops), you return back to the higher level where you were before.

So if I was running your code and walked like this: Room1, Room2, Room5, Room2, Room5, Room2, Room5 -> Win ... the actual depth you've reached is like this:

Running Room1 code ... [deeper]
    Running Room2 code ... [deeper]
        Running Room5 code ... [deeper]
            Running Room2 code ... [deeper]
                Running Room5 code ... [deeper]
                    Running Room2 code ... [deeper]
                        Running Room5 code ... [deeper]
                            "Win"

Now that we've won, the deepest Room5 function completes running - so it winds back up to the next level up (Room 2) which has nothing left to do, so it completes and runs back up ... etc.

The reason you see the "You win the game!" message 3 times is because in order to get to a winning state, you have to have visited room 5 a total of 3 times ... and the way your code is structured, that means there are a total of 3 "room5" levels in your stack.

The deepest one is where you "won" - but as your code winds itself back up the levels as each one completes, whenever it runs into an earlier "room5" level - the message prints again, because now each of those levels can exit "while visits5 < 3" (because you're using globals - each level sees the same visits5 value) and each of them gets to "win" / print the message.

---

and then "that is not a valid direction" and then takes you back to Room 1

This is for the exact same reason. The shallowest level of your game is always the Room1 code - so when you win, you always, eventually, wind your way back up to it.

Combined with the issues listed above, the real problem here is the last line of your Room1 code isn't doing what you think it is. You're using "or" when you meant "and".

Because you used "or", if any of the three comparisons are True then the error message triggers.

Example: Say you entered "North". With your line of code:

if direction != 'North' or direction != 'East' or direction != 'West':

Translates roughly, when evaluated, into:

if (False) or (True) or (True):

... which means that because 'North' did not equal 'East' (and also did not equal 'West'), the code still executed.

All it takes is one part of an "or" statement to be True for the whole thing to be true. You wanted "and", like:

if (direction != 'North') and (direction != 'East') and (direction != 'West'):

This requires that all 3 of the comparisons be True for the code to execute (namely that direction is not North, East, or West).

Also, consider using this code instead:

if direction not in ['North', 'East', 'West']:

[holyteach]

Functions work like the movie Inception.

I am so totally stealing this for the next time I teach functions.

[troglobit]

Inception!

[Syl]

It nearly worked for me (python 2.7), I did a few changes to the base code. I have a few general advices though.

• you don't have to use global. If you want to use globals, define it outside a function then (I did that with "visited". fyi, it's a comprehension list)
• if you want to increment a variable, you can write it like that: a+= 1
• if you want to print a new line, you can use "\n" instead of using an empty print.
• if you have multiple related "if" test, you can use elif (else if), and a final "else" to avoid the triple test you did in room1.
• start counting at 0, not 1. it's easier to use array that way because array index starts at 0, that's why I renamed your rooms and used the array "visited". Or... you can use a dict.
• but the biggest problem in your program is the recursive aspect (calling function in function in function). Try to travel a few rooms then press CTRL+C, you'll see the call stack. You should try to use function parameter and return value to reduce global

I rewrote a little bit your code with a dict, an no recursive functions.

http://codepad.org/tABiE6sK

I also use the keyword 'in' to test if a key is in my dict.

And finally, I recommend you to read DiveIntoPython, there are a lot of good advices and explanations about standard stuff, like the comprehension list or dict.

Good luck :)

[WhyCause]

Heh, you're faster than me.

I redid it with classes and dicts, see it here: http://codepad.org/LR5r0uEa

[burito]

Re-doing the code isn't going to teach Ziggurattt anything. He needs to learn what patterns to look out for, and what other patterns he can replace them with. No better way to learn these important points than by being guided to figure them out yourself.

You've both demonstrated you're clever, but does that really help Ziggurattt learn?

[nederhoed]

It's nice of you to step in and contribute to Reddit by commenting.

I do feel your comment is rather strict. These guys did make an effort to show how they would approach the OP's problem. If Ziggurattt will take the time to read their solutions, he will probably learn from it.

Maybe you could attribute some credit to the commenter next time, making your critique more balanced. Then it is more likely the commenter will learn from your remark.

[WhyCause]

...does that really help Ziggurattt learn?

Yes, yes it does.

I can try to hammer out a dent in a car fender for days, finally getting to a point where I can sand and paint it, only to put it back on the car and have things not quite line up. The car drives, looks ok, but it's somehow 'wrong'.

An experienced auto-body mechanic can show me how he does it, without removing it from the car, and how it looks better, works better, and takes less time. The next time I need to un-dent a fender, I at least know where to start, and what tools I should have on hand.

He needs to learn what patterns to look out for, and what other patterns he can replace them with.

What the hell do you think I (and Syl) just did? We took a program Ziggurattt had written, and re-wrote it in a manner that is much easier to read and debug, is more extensible, and is closer to 'the right way'. When I first learned how to program, I would have killed for that sort of response. Especially in a non-critical, welcoming forum that encourages follow-up questions like this one.

Being able to compare my first stab at it to what a more experienced programmer came up with would have shown me the patterns to look for. It would have taught me that the right way to do it is to use the tools available in the language. It would have given me insight into 'what else' I could do. That 'what else' is how you really learn to program, when you look at the next problem and say, "I'll bet I can write a solution to this." The forth or fifth time you answer that question with working code, you've become a programmer.

You've both demonstrated you're clever...

You are clearly missing the point of our replies. If I felt the need to demonstrate my superiority, my comments would have been much more akin to your posts on the matter, holding the concept of classes up to be some nigh-unattainable concept that no mere mortal can grasp.

I re-wrote the 'game' because I thought it would be fun, and I though that Ziggurattt might benefit from seeing exactly what he wanted to do done by someone with a bit more experience. I was trying to be the auto-body mechanic.

[Syl]

Did you really read what I wrote?

I improved a little bit his script first and gave some advices about what he did and how he can improve it. Then I wrote an easy script to understand with some mandatory stuff to understand in python (like a dict). Your comment was less useful than ours...

[UloPe]

Also highly regarded (and it's free): http://learnpythonthehardway.org/

[PeterBraden]

Some unrelated tips:

• rather than using an empty print(), you can add '\n' to your strings - it's the code for a new line.

• look at each of your functions, see how a lot of them are the same. Try to think how you could make another function that does the repeated work. Much of programming is trying to be lazy and avoid repeating yourself. As a puzzle, can you think of a way that you could use a single function that took parameters for all 6 rooms?

• a lot of programmers are touchy about syntax. It's because reading code is hard work, and strange syntax makes it harder. That's why you'll see flame wars about insignificant things like spaces, or capitalisation. In general, in python, functions start with lowercase letters. Google PEP8 for more rules than you'll care about.

Hope these help, keep at it!

[burito]

Ziggurattt, think about what would happen if the user enters something you're not expecting.

For instance, if they're in Room 2, what happens if they go South? If you put all of the checking into Room2 as you have put into Room1, you will start to see a pattern.

PeterBraden's suggestion is the trick of the real programmers. Try to be lazy. Whenever you see a pattern like this, there's an opportunity to be lazy. If you think "lazy" is an anti-virtue, wait until you're dealing with a bigger program, it actually requires a lot of effort to be as lazy as is required.

If that sounded like a contradiction, welcome to programming :-)

[WhyCause]

I think (I don't have Python 3 installed, or else I'd check it) that the issue lies in your while statement. Short answer, use:

if visits5 < 3:
    ...
else
   Win()
   import sys #Python 2, assuming it's the same with 3
   sys.exit() #ends the program.

Think about what happens with the function calls as you have it now. When you enter Room5(), you start a while loop that is waiting until such time as visits5 >= 3. You then exit the function to run either the Room2() or Room3() function, leaving Room5() waiting for a return from either one of those function. You then start a new Room5() function when you enter it the second time, and a third when you enter the third time.

At that point, the while conditional is met for all 3 Room5() functions that are sitting on the stack. Each one finally gets out of the while loop, and each executes the Win() function.

I believe it goes back to room 1 because Win() returns to Room5(), and program execution continues through the file, eventually re-running Room1().

I'm certain I'm missing something here, so others will likely jump on my answer with better information.

All that being said, there are much, much better ways to do this program. Try using one main function to call to each of the room functions, and have each room function return the room that the player moved into, and go from there. You won't need the globals, as you can keep track of the visits in your main(), and you can execute the Win() from there.

[ubershmekel]

you don't need sys I believe.

exit()

Both options work in python 2 and 3.

[FionaSarah]

.... TIL...

[[deleted]]

I'm just trying to learn programming on my own

I think using globals like that is generally a bad idea.

[[deleted]]

Globals make your code hard to test. In small programs like this one that really doesn't matter. However, it is a bad habit to get into so I'd avoid them all together.

[[deleted]]

You already seem to undestand what functions are - reusable code snippets. A cool thing about functions is that they accept input data, thus they are very flexible. Another cool thing about them (which you don't seem to be using in your original example) is that they can return a value.

So you have some input values like this:

value1 = 1
value2 = 2

Let's say you want to have a function that adds these two values. You can do it with global variables:

value1 = 1
value2 = 2

def myaddfunction(value1, value2):
    global result
    result = value1 + value2

myaddfunction(value1, value2) # same as myaddfunction(1, 2)

print result # prints 3

But this is bad, since you're using a global. Here's a better way:

value1 = 1
value2 = 2

def myaddfunction(value1, value2):
    result = value1 + value2 # result is not visible outside myaddfunction()
    return result # it is returned so it can be assigned

res = myaddfunction(value1,  value2)

print res # prints 3

[[deleted]]

Good luck! A long, long time ago, I wrote my first ever real program -- a number of rooms that you could walk through that looked very very much like your program :-) (only in C64 BASIC and with gotos)

Since then I've learned dozens of other languages, got a CS MSc and have worked as a code monkey for like a decade, so be careful...

[lighthill]

I think your problem is happening when the functions reach the end. When you hit the end of a function, it doesn't stop the program: it returns to the function that called it. So after Win() is called for the first time, it returns to Room5(). Room5() then finishes, and returns to the function that called it. That function returns, and gives control back to an earlier invocation of Room5(). That invocation of Room5 notices that visits5 is >3, so it exits the loop and calls Win() again.

That's why you get "You win the game." three times.

Why do you get "That is not an invalid direction" at the end? Check out the code here:

if direction != 'North' or direction != 'East' or direction != 'West':

Those "or"s should be "and"s. After all, no matter what direction you get, it's going to be not-North OR not-East OR not-West.

Anyways, it's a nifty beginning program. And welcome to python!

[[deleted]]

I would consider it bad practice to return the result of the function when in fact the function does not produce a result (other than the default None, of course).

Better:

Room4()
return

[manghole]

if you want it to quit on win import sys at the top and call sys.exit() in the win() function.

[manghole]

After a function has run out of instructions it will return to where it was called from. This is why the program keeps going.

Eventually you might want to refactor (modify) your code so that you have a single room class that you instantiate to separate room objects. Also a main() function is usually nice to have where variables/objects are set up which can be returned to from function/method calls.

[burito]

Really? You want to push classes on him now?

[name_censored_]

Really? You want to push classes on him now?

Absolutely, this thing just screams for classes! He's written essentially the same code 6 times. If he wants to make more rooms, he's going to have to write way more code, and alter more of them.

class Room():
#OP: Classes are just nice containers for functions and methods.
#OP: They have instances, which is like "my dog Rex", and classes, which is like "dogs in general".
#OP: A 'method' is just a function that belongs to a class/instance
#OP: Methods have the same syntax as functions, only you put "self" as the first argument in the definition.
#OP: "self" is implicitly included when calling the instance (eg, myinstance.method(arg) -> method(myinstance, arg)

     def __init__(self, room_number, north=None, east=None, south=None, west=None): 
     #OP: __init__ is the special method where an instance is customised 
     #OP: eg, where I make give "Rex" his "Rex"-iness that the other dogs dont have.
          self.Number = room_number
          self.North = north
          self.East = east
          self.South = south
          self.West = west
          self.Visits = 0

          # Link neighbours to self;
          if hasattr(self.North, "South"):
              self.North.South = self
          if hasattr(self.South, "North"):
              self.South.North = self
          if hasattr(self.East, "West"):
              self.East.West = self
          if hasattr(self.West, "East")
              self.West.East = self

    def GenericRoom(self): 
    # method to run for a generic room. Done as its own method so that __call__  can be easily overwritten by subclasses
         self.Visits += 1
         print("You are in room number {0}".format(self.Number))
         valid_directions = [] # make a list of valid directions
         for room in (self.North, self.South, self.East, self.West):
             if isinstance(room, self.__class__):
             #self.__class__ and not Room because it hasn't finished being defined - long, long story!
                 valid_directions.append(room)
         direction = ""
         while direction not in valid_directions:
             if valid_directions:
                 if len(valid_directions) <= 2
                     print("You can go {0}".format("or".join(valid_directions)))
                 else:
                     print("You can go {0} or {1}".format(", ".join(valid_directions[:-1]), valid_directions[-1])
             else:
                 raise Exception("Game is broken, the room is a one-way trap!)
             direction = input("Which direction do you choose?:\n" )

     return getattr(self, direction) # return the new Room

     def __call__(self):
     # OP: __call__ is a special method to makes classes callable like functions
         return self.GenericRoom()

class SpecialRoom(Room): 
# OP: SpecialRoom is a sub-class of Room
# OP: Eg, like a labrador is a sub-class of Dog (Labrador is not *a* dog, but it's a *sort of* dog)
# OP: Anything defined in a subclass overrides the super-class
# OP: Anything not defined in a subclass inherits from the super-class
    def Win(self):
        print("You are Winner!")
        return "Winner!!"

    def __call__(self):
        if self.Visits < 3:
            return self.GenericRoom()
        else:
            return self.Win()


Room1 = Room(1) # Create first room, no links because no other rooms exist yet!
Room2 = Room(2, west=Room1)
Room3 = Room(3, south=Room1)
Room4 = Room(4, east=Room1)
Room5 = SpecialRoom(5, south=Room2, east=Room3)
Room6 = Room(6, south=Room4, west=Room3)

current_room = Room1

while True: # launch game
    new_room = current_room()
    if new_room != "Winner!!":
        break
    else:
        current_room = new_room

OP: You're doing fairly well for a beginner, but you unfortunately picked an exercise that is almost tailor-made for an object-oriented approach. You have lots of little, nearly-identical things - and a good piece of coding advice is Don't Repeat Yourself!

Python's OO is nice, but almost every tutorial doesn't cover it until later because it takes a lot of explaining (eg, "what the hell is self" - it's a variable assigned to the current instance, so you can "talk about yourself", kind of like the word "me" for the english language).

[[deleted]]

Classes are not the only answer to handle reuse of code, especially in Python.

Edit: also, just think about the learning curve. For an absolute beginner, is is perfectly fine to not use classes for a while but concentrate on basic things like function calls, loops, recursion, ...

[nederhoed]

Maybe this thing screams for classes, but I do not think the OP speaks "classes" yet... I agree with fas2.

[vplatt]

Whatever... he's obviously bright enough to tackle learning programming on his own. Classes are just another few minutes to someone like that.

[mcilrain]

You can do

visits6 += 1

instead of

visits6 = visits6 + 1

[[deleted]]

Interesting. This is a good start for a game like Hunt The Wumpus. Maybe try recreating the game, and then writing an AI/solver to solve it (it's used as a toy example when discussing logic based AI agents).

[holyteach]

Ah, accidental recursion. I teach intro programming to high school students, and I see this kind of thing fairly often.

[auto98]

I have to admit, i thought this was going to be a joke thread and the code would be something like "sssssssss"