Removing a class from a list

Question

I am trying to remove classes from a list based on their hp. I am making a large scale battle simulator for a D&D campaign. Its a simple program that makes two lists of classes and pits them against each other.

I am running into a problem when it comes to removing dead fighters. It works fine if one fighter dies in a round, but when multiple die, it goes wonky.

def check_human_deaths():
    for i in range(len(goodguys)):
        if goodguys[i].hp <= 0:
            print('{} has died...'.format(goodguys[i].name))
            goodguys.remove(goodguys[i])

Removing the dead fighter changes the length of the list, throwing an index error:

IndexError: list index out of range

I am not to sure how to proceed with removing the dead from the battlefield. Any tips are appreciated. Let me know if I am going about this in a fundamentally wrong way.

http://stackoverflow.com/questions/1207406/remove-items-from-a-list-while-iterating-in-python — Padraic Cunningham, Feb 29 '16 at 22:35

score 3 · Answer 1 · answered Feb 29 '16 at 22:36

3

Two choices:

Modify a copy of the list and use the result of that as the new list:

>>> lst = [1,2,3,4,5]
>>> for i in lst[:]:
...     if i % 2:
...         result.append(i)
...
>>> lst = result
>>> lst
[1, 3, 5]

Modify the list in place, but do so in reverse to avoid messing up the indexes:

>>> lst = [1,2,3,4,5]
>>> for i in lst[::-1]:
...     if not i % 2:
...         lst.remove(i)
...
>>> lst
[1, 3, 5]

answered Feb 29 '16 at 22:36

bgporter

35,114
8
59
65

`lst[:] = [...]` would be a better alternative – Padraic Cunningham Feb 29 '16 at 22:37
Sorry, I'm being dense at the moment -- how so? – bgporter Feb 29 '16 at 22:38
More efficient, although the OP may want to print as they go, in that case `for i in reversed(lst)` will at least avoid a copy. – Padraic Cunningham Feb 29 '16 at 22:41
@PadraicCunningham I still don't understand what makes `lst[:] = [..]` more efficient? Thanks in advance :) – Michal Frystacky Feb 29 '16 at 23:05
@MichalFrystacky, every time you remove an element the list elements get shifted hence the OP's problem and why mutating the list you are iterating over is a bad idea, using `lst[:] = [..]` is analogous to using list.remove as it changes the original object while keeping harbouring the the efficiency of a list comp. – Padraic Cunningham Feb 29 '16 at 23:10

score 1 · Answer 2 · answered Feb 29 '16 at 22:33

1

The problem is that when you remove from goodguys, the index is reduced by one. Ex:

1,2,3,4

Remove 2

1,3,4

Index of three has been decremented by one and the size has been decremented by one.

answered Feb 29 '16 at 22:33

Robert Jacobs

3,266
1
20
30

bddap · Answer 3 · 2016-02-29T22:52:59.927

1

goodguys = [ guy for guy in goodguys if guy.hp > 0 ]

This will filter out any dead goodguys in your array.

You can use it in your function like this:

def check_human_deaths():
  global goodguys
  dedguys = [ guy for guy in goodguys if guy.hp <= 0 ]
  for guy in dedguys:
      print('{} has died...'.format(guy.name))
  goodguys = [ guy for guy in goodguys if guy.hp > 0 ]

edited Feb 29 '16 at 22:52

answered Feb 29 '16 at 22:44

bddap

529
4
8

Moinuddin Quadri · Answer 4 · 2016-02-29T22:41:47.873

0

You are getting this error because you are iterating over the length of the list let's say l. Each time you call list.remove(), length of the list is decreased by 1, and your program gives index error because list[l] does not exists. Instead you can use:

def check_human_deaths():
    for goodguy in list(goodguys):
        if goodguy.hp <= 0:
            print('{} has died...'.format(goodguy.name))
            goodguys.remove(goodguy)

Note: list(goodguys) will create the copy of goodguys list preventing the weird behavior of for loop when object is removed from the list:

>>> t= [1,2,3]
>>> x= list(t)
>>> x.remove(1)
>>> x
[2, 3]
>>> t
[1, 2, 3]

Even after removing the value from x, this value will still be present in t and your for for loop will not behave weirdly

edited Feb 29 '16 at 22:41

answered Feb 29 '16 at 22:32

Moinuddin Quadri

46,825
13
96
126

You are correct. I haven't checked if `list()` performs a deep copy of the objects or just copies the references and the possible issue of the `global` `goodguys`, but my last point was incorrect. – Michal Frystacky Feb 29 '16 at 22:47
1

Just to correct you, `list()` doesn't performs `deep copy` instead it performs `shallow copy`. The class objects within the list will still remain the same. `list()` is equivalent of `copy.copy()` where as for deep copy in python we have `copy.deepcopy()` (which will even create the copy of class objects within the list). – Moinuddin Quadri Feb 29 '16 at 22:55
Less of a correction and more of a clarification. Thanks :) And a solid answer to the overall question. – Michal Frystacky Feb 29 '16 at 23:02
I did. :) Now only the downvoter has to revise his as well. – Michal Frystacky Mar 02 '16 at 01:54

score 0 · Answer 5 · answered Feb 29 '16 at 22:42

As others said, You can't change the list while iterating over it because weird things will happen. Instead You can save elements that You want to delete and delete them after the loop:

def check_human_deaths():
    things_to_delete = []
    for i in range(len(goodguys)):
        if goodguys[i].hp <= 0:
            print('{} has died...'.format(goodguys[i].name))
            things_to_delete.append(goodguys[i])
    goodguys = [x for x in goodguys if x not in things_to_delete]

score 0 · Answer 6 · edited May 23 '17 at 12:23

You can do it using a generator function to allow you to print and update the list in a more efficient and correct manner doing a single pass over the list:

class Foo():
    def __init__(self, hp, name):
        self.hp = hp
        self.name = name


def check_human_deaths(goodguys):
    for guy in goodguys:
        if guy.hp < 1:
            print('{} has died...'.format(guy.name))
        else:
            yield guy

Demo:

In [29]: goodguys = [Foo(0,"foo"), Foo(1,"bar"), Foo(2,"foobar"), Foo(-1,"barf")]

In [30]: goodguys[:] = check_human_deaths(goodguys)
foo has died...
barf has died...

In [31]: 

In [31]: print(goodguys)
[<__main__.Foo instance at 0x7fdab2c74dd0>, <__main__.Foo instance at 0x7fdab2c6e908>]

This answer has a very good explanation as to why you should never mutate a list you are iterating it.

Also if you are going to start at the end of the list and use remove, you can use reversed instead of creating another copy of the list:

   for guy in reversed(goodguys):
        if guy.hp <= 0:
            print('{} has died...'.format(guy.name))
            goodguys.remove(guy)

But that is not going to be as efficient as the first option.

Removing a class from a list

6 Answers6