Multiple dictionaries in list don't properly work with pop method

Question

With the following code I'm trying to append all cards form the deck to the hand that have the "Cool" Key in their dictionary.

But for some some reason it only draws 2 out of 3 attacks. Can anybody explain to me what is happening?

attack = {"Name":"Attack","Cool":True}
defend = {"Name":"Defend"}

deck = [attack,attack,defend,defend,defend,attack]

hand = []

for card in deck:
    try:
        if card["Cool"] == True:
            hand.append(deck.pop(deck.index(card)))
    except Exception as e:
        print (e)

print(hand)
#this Print will show that it has only appended 2 of the 3 attack dictionaries.

I've also tried this with a while loop thinking that I have messed something up with the index logic but to no avail:

EDIT: So thanks to @Chrispresso I was able to wrap my head around how to make the while loop work! I've edited that part in the code

i = 0
while i < len(deck):
    try:
        if deck[i]["Cool"] == True:
            hand.append(deck.pop(i))
            i += 1 #commenting out this line does the trick as you should not be increasing the index when you are popping something from the list as this basically makes you jump two places instead of one.
    except Exception as e:
        i += 1
        print (e)

Thanks so much for your help in advance! EDIT: So the desired output would be

print(hand)
#[{"Name":"Attack","Cool":True},{"Name":"Attack","Cool":True},{"Name":"Attack","Cool":True}]

but currently the output is:

print(hand)
#[{"Name":"Attack","Cool":True},{"Name":"Attack","Cool":True}]

These are all aliases of the same two objects. Did you mean to make copies? Also, it's important to not modify a list when iterating over it -- you might try `for card in deck[:]` to iterate on a copy, because each pop changes the length and elements are skipped. What are you trying to achieve here (please show exact output)? See also [Modifying list while iterating](https://stackoverflow.com/questions/1637807/modifying-list-while-iterating) — ggorlen, Aug 29 '21 at 23:38
Ah, I might lack some understanding there. Do you know how I could go about this sensibly? — Difio, Aug 29 '21 at 23:39
You should not be iterating over the same list you're popping from . Bad things happen when you do that. For the while-loop, you're incrementing `i` even though you're decreasing the length, which once again.... bad things happen — Chrispresso, Aug 29 '21 at 23:41
You just want something like `han = [d for d in deck if d.get('Cool')]` — juanpa.arrivillaga, Aug 29 '21 at 23:47

martineau · Answer 1 · 2021-08-30T00:21:00.567

1

The problem is you are iterating over the thing you are changing. One way to avoid it in this case is simply by making a copy and iterating over that:

attack = {"Name":"Attack","Cool":True}
defend = {"Name":"Defend"}

deck = [attack,attack,defend,defend,defend,attack]

hand = []

for card in deck[:]:  # ITERATE OVER A COPY.
    try:
        if card["Cool"] == True:
            hand.append(deck.pop(deck.index(card)))
    except Exception as e:
        print (e)

# This will now show that hand has all 3 attack dictionaries.
print(hand)  
# And this shows they were removed from the deck.
print(deck)  # [{'Name': 'Defend'}, {'Name': 'Defend'}, {'Name': 'Defend'}]

edited Aug 30 '21 at 00:21

answered Aug 29 '21 at 23:46

martineau

119,623
25
170
301

@IoaTzimas: Yeah they are. See the `deck.pop`. – user2357112 Aug 29 '21 at 23:58
@martineau Yes you are right, sorry – IoaTzimas Aug 29 '21 at 23:59

IoaTzimas · Answer 2 · 2021-08-29T23:54:42.880

0

Try the following:

attack = {"Name":"Attack","Cool":True}
defend = {"Name":"Defend"}

deck = [attack,attack,defend,defend,defend,attack]

hand = []

for i in range(len(deck)-1, -1, -1):
    try:
        if deck[i]["Cool"] == True:
            hand.append(deck[i])
            deck.pop(i)
    except Exception as e:
        print (e)
hand=hand[::-1]

Output:

>>>print(hand)
[{'Name': 'Attack', 'Cool': True}, {'Name': 'Attack', 'Cool': True}, {'Name': 'Attack', 'Cool': True}]
>>>print(deck)
[{'Name': 'Defend'}, {'Name': 'Defend'}, {'Name': 'Defend'}]

edited Aug 29 '21 at 23:54

answered Aug 29 '21 at 23:41

IoaTzimas

10,538
2
13
30

No, you should loop directly through the list, not a range – juanpa.arrivillaga Aug 29 '21 at 23:46
I think he wants to remove them from the deck if they are in a players hand. – Alex Stoneham Aug 29 '21 at 23:51

score 0 · Answer 3 · answered Aug 29 '21 at 23:49

As the comments from @ggorlen and @Chrispresso mention, you should not mutate the collection while iterating the collection in this way.

attack = {"Name":"Attack","Cool":True}
defend = {"Name":"Defend"}

deck = [attack,attack,defend,defend,defend,attack]

    hand = []
    indexes_to_remove = []
    for card in deck:
       try:
           if card["Cool"] == True:            
               indexes_to_remove.append(deck.index(card))
               hand.append(card)
       except Exception as e:
           print (e)
    for deck_index in indexes_to_remove:
       try:
           deck.pop(deck_index) 
       except Exception as e:
           print (e)

score 0 · Answer 4 · answered Aug 29 '21 at 23:51

Instead of using the indexing syntax([]), use the dict.get(key) function. This assures that a KeyError won't be thrown (and returns None by default when the key isn't found):

Then this can be achieved with a simple list filter:

attack = {"Name":"Attack","Cool":True}
defend = {"Name":"Defend"}

deck = [attack,attack,defend,defend,defend,attack]
hand = [card for card in deck if card.get("Cool")]

Multiple dictionaries in list don't properly work with pop method

4 Answers4