0

My code:

ipList = ["192.168.0.1", "sg1234asd", "1.1.1.1", "test.test.test.test"]
blackList = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z", "_", ","]
for ip in ipList:
    for item in blackList:
        if (item in ip) == True:
            ipList.remove(ip)
        else:
            pass
print ipList

From what I can read of this code, it should print only print ipList elements 0 and 2, why do I get given ValueError: list.remove(x): x not in list in line 6?

codaamok
  • 717
  • 3
  • 11
  • 21
  • Basically: [How to validate IP address in Python?](http://stackoverflow.com/questions/319279/how-to-validate-ip-address-in-python) – Ashwini Chaudhary Aug 06 '15 at 15:38
  • @Ashwini, your first link was perfect and helped me understand why I shouldn't remove items from lists that I'm iterating and suggested that I should put elements that do qualify in a seperate list and use that one instead – codaamok Aug 06 '15 at 15:41

1 Answers1

6

You have two issues, you should not iterate over and remove elements from a list, you should also break the inner loop when you find a match, you cannot remove the same element 28 times unless you have it repeated 28 times:

ipList = ["192.168.0.1", "sg1234asd", "1.1.1.1", "test.test.test.test"]
blackList = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z", "_", ","]
for ip in reversed(ipList):
    for item in blackList:
        if item in ip:
            ipList.remove(ip)
            break

print ipList

Without breaking, after you find a match for the ip, you still potentially search 20+ times for another match for the ip even though you have already removed it so if you get another match you are going to try removing it again.

You can use anywith your loop, any will short circuit on finding a match in the same way the break is doing above:

for ip in reversed(ipList):
     if any(item in ip for item in blackList):
         ipList.remove(ip)

Which can simply become:

ipList[:] = [ip for ip in ipList if not any(item in ip for item in blackList)]
Padraic Cunningham
  • 176,452
  • 29
  • 245
  • 321
  • shouldnt an else also be included for when item is not in ip? – awbemauler Aug 06 '15 at 15:40
  • 1
    @awbemauler nope, in the list comp we only keep valid strings, in the loop we only remove bad strings – Padraic Cunningham Aug 06 '15 at 15:48
  • I was smashing my head against a poll for not understanding the logic. Decided to sleep on it and came back this morning fully understanding. Thank you for your answer – codaamok Aug 07 '15 at 07:47
  • Due to the nature of the characters of `[:]` ... can you explain what `[:]` means please? Can't Google or find other questions on SO regarding it. **Edit:** Is it called extended slices? – codaamok Aug 07 '15 at 07:54
  • Also, did you use `reversed()` because it uses object references thus quicker and safer for massive lists? Or was there logic behind the reason i.e. the specific ordering of the list? http://effbot.org/pyfaq/how-do-i-iterate-over-a-sequence-in-reverse-order.htm – codaamok Aug 07 '15 at 08:16
  • @adampski,using `[:]` means we modify the original object/list so we get to use the efficiency of a list comp instead of using a regular for loop. we could have copied the list but using reversed is more efficient memory wise allowing us to also safely mutate the list as we iterate over the elements in reverse order – Padraic Cunningham Aug 07 '15 at 09:19