3

I can't figure out why this section of code is not removing "tactic" (tuples) from a List[of tuples()]

def _cleanup(self):
    for tactic in self._currentTactics:
        if tactic[0] == "Scouting":
            if tactic[1] in self._estimate.currently_visible:
                self._currentTactics.remove(tactic)
        elif tactic[0] == "Blank":
            self._currentTactics.remove(tactic)
        elif tactic[0] == "Scout":
            self._currentTactics.remove(tactic)

Screenshots of my IDE (pydev) with further debugging info is available at: https://i.stack.imgur.com/VP0Fw.jpg

EDIT: A bug fix I noticed and an improvement. To clarify, "Blank" is getting removed, "Scouting" is getting removed when necessary, and "Scout" tactics are NOT getting removed afaik.

Vikram Saran
  • 1,143
  • 8
  • 17
  • 1
    You're modifying the list you're iterating over ... see [this post](http://stackoverflow.com/q/1207406/589206) for a problem description. – hochl Apr 24 '12 at 11:30
  • Iterating over changing collection (especially when elements are deleted) is rarely a good idea. It usually leads to unexpected behavior. – Fenikso Apr 24 '12 at 11:31
  • `for tactic in copy.copy(self._currentTactics):` may be a quick fix. – Fenikso Apr 24 '12 at 11:33

1 Answers1

3

You are removing members from the list while you are iterating over it. By doing this you will miss certain elements in the list. You need to iterate over a copy of the list instead.

Change:

for tactic in self._currentTactics:

to:

for tactic in self._currentTactics[:]:
fraxel
  • 34,470
  • 11
  • 98
  • 102
  • I would favor `copy.copy()` for being more explicit and descriptive than list slice. – Fenikso Apr 24 '12 at 11:36
  • [:] is working a lot better. Although I have a whole bunch of kinks I'm still working out this is much easier to deal with already. I'm not sure what @Fenikso means by copy.copy() though? – Vikram Saran Apr 24 '12 at 11:39
  • @Vikram Saran - Glad to have helped. `[:]` and `copy.copy()` do the same thing, but Fenikso prefers `copy.copy()` as it is more 'readable' and so perhaps easier to understand that it is doing a copy than using `[:]`, (which is called a list slice by the way). – fraxel Apr 24 '12 at 11:44