Why can I not delete list elements while iterating in an OO setting?

Question

My situation is as follows: I'm working on an implementation of BlackJack. I've got classes for Card, Hand, Player, Deck, and Game. The main game object stores players and a deck, while players stores hands which store cards.

I often do something like the following. In this example I am dealing the initial cards to each player.

num_cards = 2
for player in self.players:
    new_hand = Hand()
    for i in range(num_cards):
        new_hand.add_card(self.deck.get_next_card())
    player.assign_hand(new_hand)

This works splendidly. My problem now is that I wish to delete a hand from a player's set of hands (a player can split in BlackJack, causing more than one hand to be generated). In the following function, I intend to loop through each player's hands. If the value of the hand is greater than 21, I want to delete the hand. (Note that the remove() functionality below is normally performed in the Player class, called via a Player method named fold_hand(). I was having the same problem, so I have moved the code to somewhere more visible for expository purposes.)

for player in self.players:
    for hand in player.hands:
        if hand.smallest_value() > 21:
            player.hands.remove(hand)

This does not work. To be clear, I am able to print out the hand before the remove() line and it does not print out after. That is, it seems to be removed. However, the in the next turn of play, the hand is back again. Thus the players' hands grow every turn.

The above code is in a function called validate_player_hands() in the Game class. This function is called from a file called play.py, which exists to start/end the game and facilitate the primary game loop. Thus, the only call to validate_player_hands() is in the play.py file, one indent in, in the game loop. I call:

game.validate_player_hands()

I have also tried finding the index of the hand and using the 'del' keyword, but the result is the same.

Why would the list element (a Hand object in a list called player.hands) fail to delete when it looks like it has been deleted?

Thanks in advance,

ParagonRG

It's a bad idea to modify lists that you're iterating through. — Waleed Khan, Apr 09 '12 at 19:46
You can't delete an item from a collection you are iterating over. It's kind of like cutting the branch under your feet when cutting a tree. — Mihai Oprea, Apr 09 '12 at 19:47
@MihaiOprea Uhh, yeah, that does make a whole lot of sense. I'm not sure how I missed that. I'll have to find another way to delete the hands while keeping the code clean. Thanks. — Paragon, Apr 09 '12 at 19:52
@Paragon: oh ok. What kind of data structure is `hand` in this situation? Is it a class? — Joel Cornett, Apr 09 '12 at 19:53
@JoelCornett Yes, it is a class. I describe the various classes I created at the top of the question. — Paragon, Apr 09 '12 at 19:57

Joel Cornett · Answer 1 · 2012-04-09T20:16:13.257

3

How about using a simple list comprehension to eliminate hands:

for player in self.players:
    player.hands = [hand for hand in player.hands if hand.smallest_value() <= 21]

EDIT

With filter:

for player in self.players:
    player.hands = filter(lambda x: x.smallest_value() <= 21, player.hands)

edited Apr 09 '12 at 20:16

answered Apr 09 '12 at 20:00

Joel Cornett

24,192
9
66
88

1

Yes, and I think the filter() function works very well in the manner. Yours, however, is very compact. – Paragon Apr 09 '12 at 20:06
1

@Paragon: I did a little benchmarking, and the list comprehension seems to be about 4% faster with a list of 7 elements. Not sure how much that applies and/or matters in your case, however. – Joel Cornett Apr 09 '12 at 20:28
@JoelCornett More than anything else, I'm after a combination of brevity, readability, and overall cohesion on the code. Performance won't be much of an issue. Nevertheless, I appreciate your work here. – Paragon Apr 09 '12 at 20:34

luke14free · Accepted Answer · 2012-04-09T20:07:14.630

2

Create a copy and iterate over the the object with an index tied to the object length. Set every elements to be deleted with a 0 and then filter the hands to purge the zeros.

for player in self.players:
    for hand_idx in range(len(player.hands)):
        if player.hands[hand_idx].smallest_value() > 21:
            player.hands[hand_idx]=0
    player.hands=filter(None,hands)

edited Apr 09 '12 at 20:07

answered Apr 09 '12 at 19:52

luke14free

2,529
1
17
25

This will generate an IndexError if a hand is deleted as the indexes in `hands` will shift down if an element is deleted – Joel Cornett Apr 09 '12 at 19:54
yep sorry code was incomplete, the edit should fix the error as the original list is never touched now. – luke14free Apr 09 '12 at 20:00
Line 5 seems off to me. `hands_copy[hand_idx].remove(hands[hand_idx])` seems like it should instead be `hands_copy.remove(hands[hand_idx])`. Otherwise, if at least two elements from hands_copy are deleted, the element hands[hand_idx] may not be the same as hands_copy[hand_idx] by the second delete. – Paragon Apr 09 '12 at 20:05
@Paragon, edit now should be more efficient as is not using the copy anymore. It just puts a 0 instead of the node inside hands and then purges all the 0s. I'm setting to 0 and not deleting to avoid index shifting issues. – luke14free Apr 09 '12 at 20:10
This is very cool. I did not know of the power of using filter() with None as its function! – Paragon Apr 09 '12 at 20:16
of course! if you use itertools.ifilter it gets even faster as it's backed by a generator. remember that whenever you use lambdas your code slows donw. With None you go waay faster. – luke14free Apr 09 '12 at 20:18

score 0 · Answer 3 · answered Apr 09 '12 at 20:01

You will want to do something like the following:

newList = hands[:]   
for hand in newList:  
    if hand.smallest_value() > 21:  #your code here
            player.hands.remove(hand)

This allows you to make a copy of the list to be modified while iterating over that copy thus avoiding the "tree limb" scenario that Mihai mentioned.

score 0 · Answer 4 · edited May 23 '17 at 11:49

This can be solved by adding [:] to the end of the "for" statement. This creates a copy of the list. You can then loop over the copy, while changing the original list:

for player in self.players:
    for hand in player.hands[:]:
        if hand.smallest_value() > 21:
            player.hands.remove(hand)

(This is Python's list slicing syntax. It's one of the fastest way to copy a list. More often this syntax is used in the form some_list[3:8] to get the list entries from index 3 to 7 inclusive, but by leaving out the first number you can get everything from the start of the list, and by leaving out the last number you get everything up to the end of the list).

Why can I not delete list elements while iterating in an OO setting?

4 Answers4