CODEWITHSUNDEEP
pythonlist
Lost_DM
Lost_DM

Reputation: 951

Failing to iterate over my list correctly

The strange thing is that I'm using Python built in iteration O.o

I have a class called Card. Card has, among other things, a name and a list of symbols (string).

Here is a piece of my code (all the prints are for debugging purposes)

    # This prints all the names of the cards in the deck before the iteration.
    print(str([card.name for card in self.thegame.game_deck.deck[0]]))

    for card in self.thegame.game_deck.deck[0]:
        if 'CASTLE' not in card.symbols: 
            self.thegame.game_deck.deck[0].remove(card)
            print(card.name + ' was removed')
        else: print(card.name + ' was not removed')

    # This prints all the names of the cards in the deck after the iteration.                
    print(str([card.name for card in self.thegame.game_deck.deck[0]]))

And strangely enough, this is the output on stdout:

['Writing', 'Tools', 'The Wheel', 'Sailing', 'Pottery', 'Oars', 'Mysticism', 'Me
talworking', 'Masonry', 'Domestication', 'Code of Laws', 'Clothing', 'City State
s', 'Archery', 'Agriculture']

Writing was removed
The Wheel was not removed
Sailing was removed
Oars was not removed
Mysticism was not removed
Metalworking was not removed
Masonry was not removed
Domestication was not removed
Code of Laws was removed
City States was not removed
Archery was not removed
Agriculture was removed


['Tools', 'The Wheel', 'Pottery', 'Oars', 'Mysticism', 'Metalworking', 'Masonry'
, 'Domestication', 'Clothing', 'City States', 'Archery']

As you can clearly see, there are names in the first list (specifically: 'Tools', 'Pottery', 'Clothing')

That nothing was print about in the second part of the output, and indeed they are left in the list (all three, btw, have 'CASTLE' in they symbols and should be removed).

Can someone see what am I missing?

Upvotes: 1

Views: 133

Answers (5)

Raymond Hettinger
Raymond Hettinger

Reputation: 226336

Another common alternate to removing-while-iterating is to mark items for pending deletion (mutating the value in the list but not changing the list length) and then making a second high-speed pass to filter-out the unwarranted cards:

# This prints all the names of the cards in the deck before the iteration.
print(str([card.name for card in self.thegame.game_deck.deck[0]]))

for i, card in enumerate(self.thegame.game_deck.deck[0]):
    if 'CASTLE' not in card.symbols: 
        self.thegame.game_deck.deck[0][i] = None
        print(card.name + ' was marked to be removed')
    else: print(card.name + ' was not removed')
# This does the actual removal
self.thegame.game_deck.deck[0][:] = filter(None, self.thegame.game_deck.deck[0])

# This prints all the names of the cards in the deck after the iteration.                
print(str([card.name for card in self.thegame.game_deck.deck[0]]))

Upvotes: 0

Mark Byers
Mark Byers

Reputation: 838276

You shouldn't remove items from a list while you iterate over it.

Either iterate over a copy of the list:

for card in self.thegame.game_deck.deck[0][:]:
                                          ^^^ copies the list

Or create a new list with the items you want to keep and then re-assign it:

game_deck = self.thegame.game_deck
game_deck.deck[0] = [card for card in game_deck.deck[0] if 'CASTLE' in card.symbols]

Upvotes: 7

Daenyth
Daenyth

Reputation: 37441

To elaborate on what happens when you do this:

In [99]: l = range(5)

In [100]: for i in l:       
    l.remove(i)
    print list(enumerate(l))
   .....:     
   .....:     
[(0, 1), (1, 2), (2, 3), (3, 4)]
[(0, 1), (1, 3), (2, 4)]
[(0, 1), (1, 3)]

As you change the list, you're changing what index refers to which item, as you can see with the value 4 in the list above.

Upvotes: 0

Elli Amir
Elli Amir

Reputation: 365

You're removing items from the list upon which you're iterating. Create a new list with cards that have the CASTLE icon instead of removing these that have the icon.

Upvotes: 0

Daniel Roseman
Daniel Roseman

Reputation: 599610

You're modifying the list that you're iterating through. This is a bad idea. Instead, append the ones you do want to keep to a separate list, and reassign that back to the card at the end.

Upvotes: 0

Related Questions