1

I am trying to write a function, that returns a string letters that are not in lettersGuessed

words = list('abcdefghijklmnopqrstuvwxyz')

def getAvailableLetters(lettersGuessed):
    [words.remove(letter) for letter in words if letter in lettersGuessed]
    return ''.join([str(elem) for elem in words]) 

then i call my function

getAvailableLetters(['e', 's', 'i', 'k', 'p', 'r'])

But the result is different from what I expected. My output:

'abcdfghjlmnoqstuvwxyz' 

# the letter 's' in the output, but it shouldn't be there.

Correct output:

'abcdfghjlmnoqtuvwxyz' 

what am I doing wrong ?

  • 9
    Don't remove elements while iterating over it. That will leads to unexpected behaviors. You could iterate over the copy and remove the elements. ie, `for letter in words[:]:words.remove(letter)`. Also there is no need to build the list `[words.remove(letter) for letter in words[:] if letter in lettersGuessed]`. `remove ` is an inplace operation and does not yield a value. So you are actually building a list of `None` values. – Abdul Niyas P M Feb 12 '20 at 07:36
  • 2
    You're modifying a list at the same time as you're iterating it, which means you will skip some values in the list. Try something like `return ''.join(l for l in words if l not in lettersGuessed)` instead – Nick Feb 12 '20 at 07:37

3 Answers3

1

The reason is that you're modifying a list while iterating through it - this should not be done. I'll share here the advice @AlexMartelli gave in one of his answers:

Never alter the container you're looping on, because iterators on that container are not going to be informed of your alterations and, as you've noticed, that's quite likely to produce a very different loop and/or an incorrect one.

Here's an alternative solution (also using snake_case), using sets - given the assumption that neither the available letters nor the guessed letters would contain duplicates (unless by mistake):

all_letters = set('abcdefghijklmnopqrstuvwxyz')

def get_available_letters(letters_guessed):
    available_letters = all_letters - set(letters_guessed)
    return ''.join([str(letter) for letter in available_letters])

A caveat here, the above solution might not respect the order in which you specified your letters.

If you still want to stick to lists, then do it in two stages:

all_letters = list('abcdefghijklmnopqrstuvwxyz')

def get_available_letters(letters_guessed):
    available_letters = [letter for letter in all_letters if letter not in letters_guessed]
    return ''.join([str(letter) for letter in available_letters])
Jir
  • 2,985
  • 8
  • 44
  • 66
1

I will give you a small example to see what it is happening when you remove elements from a list while you iterate over:

l = ['a', 'b', 'c', 'd', 'e', 'f']
for i, c in enumerate(l):
    print('_' * 25)
    print('iteration', i)
    print('index value', i)
    print('elemnt at index ', i, ':', l[i])
    print('list length:', len(l))
    l.remove(c)
    print('\nafter removing an element')
    print('list length:', len(l))

    print('index value', i)
    if len(l) > i:
        print('elemnt at index ', i, ':', l[i]) # this element will not be removed

print('_' * 40)
print('list after iterateion:', l)\

output:

_________________________
iteration 0
index value 0
elemnt at index  0 : a
list length: 6

after removing an element
list length: 5
index value 0
elemnt at index  0 : b
_________________________
iteration 1
index value 1
elemnt at index  1 : c
list length: 5

after removing an element
list length: 4
index value 1
elemnt at index  1 : d
_________________________
iteration 2
index value 2
elemnt at index  2 : e
list length: 4

after removing an element
list length: 3
index value 2
elemnt at index  2 : f
________________________________________
list after iterateion: ['b', 'd', 'f']

as you can see, if you remove an element while you iterate a list you modify the size of the list and from one iteration to the other, you are jumping the next element, the for loop is increasing with one the index after each iteration expecting to grab the next element but you shrink the list with one element so the for loop is actually jumping 1 element

if you want to modify the global variable words you can use:

def getAvailableLetters(lettersGuessed):
    global words
    words = [c for c in words if c not in lettersGuessed]

    return ''.join([str(elem) for elem in words])

getAvailableLetters(['e', 's', 'i', 'k', 'p', 'r'])

output:

'abcdfghjlmnoqtuvwxyz'

if you only want to return the letters that are not in lettersGuessed without modifying the global variable words:

def getAvailableLetters(lettersGuessed):
    return ''.join(c for c in words if c not in lettersGuessed)
kederrac
  • 16,819
  • 6
  • 32
  • 55
1

remove() function is decrementing the length of words, whereas, in Python, loop variable is initialized for its limits at the start of loop only. Try to understand this with a small example

lis = [1, 2, 4, 6, 5]
for i in lis:
    if i % 2 == 0:
        lis.remove(i)

This code outputs [1, 4, 5], whereas the expected output is [1, 5]. This is because, i was already initialized to go from 0 to 5 index. The first time lis.remove() removed 2, the length of lis decreased, and it got updated to lis = [1, 4, 6, 5], and now i should point to 4 (the next element), whereas i now points to 6, i.e., index 2 (because 2 was at index 1), and hence never checks for the value 4. The same is happening in your code.

For the solution, it is preferred to use while loop. This problem does not arise with while loop, but with for loops only.

Swati Srivastava
  • 1,102
  • 1
  • 12
  • 18