CODEWITHSUNDEEP
python
user3150635
user3150635

Reputation: 547

code runs else statement repeatedly

every time the below code runs, it goes straight through to the first else statement, then runs the next else statement four times. what is going on and how do I fix it so it calls the movement() module? 

class square(object):
    def __init__(self,updown,leftright,residence,name):
        self.updown = updown
        self.leftright = leftright
        self.residence = bool
        self.name = name

    a1 = square(1,1,True,"a1")
    a2 = square(2,1,False,"a2")
    b1 = square(1,2,False,"b1")
    b2 = square(2,2,False,"b2")

    square_dict = {a1:"a1",a2:"a2",b1:"b1",b2:"b2"}
    movement_select()

 def movement_select():
    response = raw_input("where would you like to move?")
    if response in square_dict:
        moveTo = square_dict[response]
    else:
        print "this runs once"
    for a in square_dict:
        if a.residence == True:
            moveFrom = a
            movement(moveFrom,moveTo)
        else:
            print "this runs four times"
movement_select()

Upvotes: 0

Views: 118

Answers (5)

Henry Keiter
Henry Keiter

Reputation: 17168

First problem: your dictionary appears to be backwards: you want to look up the square objects from their locations, rather than the other way around. This is why your first conditional is never true. You also might as well strip() the response to ensure that you don't have any hidden whitespace in there.

square_dict = {"a1":a1, "a2":a2, "b1":b1, "b2":b2}
# Later...
response = raw_input("where would you like to move?").strip()
# Later still...
for a in square_dict.values(): # Because we switched the dictionary around!

If you don't want to silently strip off the whitespace, I'd suggest that you at least echo their input back to them (print('"{}"'.format(response))) in the case that it's not found in your dictionary, so they (you) can be sure that at least the input was correct.

The second problem is because of how you define residence. You set the variable equal to bool, which is not what you want at all. Line five ought to read:

self.residence = residence

Finally, some other thoughts on your code! You check whether a value is True by checking if a.residence == True:. The preferred form of this comparison is the simpler version: 

if a.residence:

Your methods could also be named more descriptively. Generally speaking, it's always nice to begin a function or method name with a verb, to improve readability. This is of course a question of style, but for instance, the two functions we see, movement_select and movement aren't extremely clear as to their function. It would be much easier to read if they used a standardized form, e.g. choose_move and perform_move.

Upvotes: 3

abarnert
abarnert

Reputation: 365707

Look at how you're defining residence:

    self.residence = bool

So, for any square a, a.residence will be the type bool, never the boolean value True (or anything else). So this test will always fail:

    if a.residence == True:

To fix it, change that first line to:

    self.residence = residence

While we're at it, you rarely need == True, so you can also change the second line to:

    if a.residence:

But that isn't a necessary fix, just a way of simplifying your code a bit.

Meanwhile, your squares_dict is a bit odd. I'm not sure whether it's incorrect or not, but let's take a look:

It maps from square objects to their names. That could be a useful thing to do. It means you can iterate over the dict and get all the squares—as you correctly do in your code. And if you later had a square and wanted to get its name, you could use square_dict for that. Then again, you could probably get the same benefit with just a square_list, and using the name already available as an attribute of the square objects (unless you need the same square to have different names in different contexts).

And meanwhile, a mapping in this direction can't be used for looking up squares_dict[response], because response is a name, not a square. So, you definitely need a mapping in the opposite direction, either in addition to or instead of this one.

If you scrap the square-to-name mapping and only keep the name-to-square mapping, you can still iterate over the squares; you'd just have to do for square in squares_dict.values(): instead of just for square in squares_dict:.

Upvotes: 3

Ryan G
Ryan G

Reputation: 9560

self.residence = bool

should be 

self.residence = residence

Upvotes: 2

Mailerdaimon
Mailerdaimon

Reputation: 6080

You don´t set residence right, this is wrong: 

class square(object):
    def __init__(self,updown,leftright,residence,name):
        self.updown = updown
        self.leftright = leftright
        self.residence = bool
        self.name = name

it has to be

class square(object):
    def __init__(self,updown,leftright,residence,name):
        self.updown = updown
        self.leftright = leftright
        self.residence = residence  # not bool!!
        self.name = name

Upvotes: 1

volcano
volcano

Reputation: 3582

Your response contains \n symbol, just strip() it You should also swap places between keys and values in your dictionary

Upvotes: -2

Related Questions