0

I was expecting vowel free string from the code below but it doesn't give me what I'm expecting. Help please.

def disemvowel(word):
    words = list(word)
    for i in words:
        if i.upper() == "A" or i.upper() == "E" or i.upper() == "I" or i.upper() == "O" or i.upper() == "U":
            words.remove(i)


    return print(''.join(words))

disemvowel("uURII")

I was expecting the output to be 'R' but I'm getting 'URI'.

Anush Poudel
  • 45
  • 1
  • 9
  • 1
    I don't think it's a dup. He's making a completely different mistake from the OP in that question. And, assuming he wants to learn what he's doing, rather than just throw away his code and cargo-cult someone else's, that question and its answers aren't going to help him. – abarnert Mar 18 '18 at 08:33
  • @abarnert Fair point. – Mr. T Mar 18 '18 at 08:35
  • @abarnert I agree the duplicate doesn't explain it all like you did, but there are other duplicates explaining that well-known behaviour. Let me find one. – Jean-François Fabre Mar 18 '18 at 08:41
  • @Jean-FrançoisFabre The second dup (on how to remove items from a list) is definitely useful to the OP, and anyone else with his question; I would have voted to close on that one if it weren't already auto-closed. – abarnert Mar 18 '18 at 09:03
  • It's recent, and I followed that question at the time. You didn't. People are allowed to miss some dupes. Just not _all_ of them like some do, that's it :) – Jean-François Fabre Mar 18 '18 at 09:04
  • @Jean-FrançoisFabre By the way, there used to be an (off-site) collection someone set up somewhere of frequently-useful dups that was easier than searching `[python] hopefully appropriate keywords`; do you know if that's still maintained anywhere? – abarnert Mar 18 '18 at 09:05
  • that one? https://sopython.com/canon/?page=1 pretty good (but not as good as my personal collection :)) – Jean-François Fabre Mar 18 '18 at 09:05
  • @Jean-FrançoisFabre Thanks! That's even better than the one from a couple years ago (probably a newer evolution of it?); bookmaked. – abarnert Mar 18 '18 at 09:06

2 Answers2

5

Don't call remove on a list while iterating over it.

Think about what happens when you do it.

  • First, words = 'uURII', and i is pointing at its first character.
  • You call words.remove(i). Now words = 'URII', and i is pointing at its first character.
  • Next time through the loop, words = 'URII', and i is pointing to its second character. Oops, you've missed the U!

There are a few ways to fix this—you can iterate over a copy of the list, or you can index it from the end instead of the start, or you can use indices in a while loop and make sure not to increment until you've found something you don't want to delete, etc.

But the simplest way is to just build up a new list:

def disemvowel(word):
    words = list(word)
    new_letters = []
    for i in words:
        if i.upper() == "A" or i.upper() == "E" or i.upper() == "I" or i.upper() == "O" or i.upper() == "U":
            pass
        else:
            new_letters.append(i)
    print(''.join(new_letters))

This means you no longer need list(word) in the first place; you can just iterate over the original string.

And you can simplify this in a few other ways—use a set membership check instead of five separate == checks, turn the comparison around, and roll the loop up into a list comprehension (or a generator expression):

def disemvowel(word):
    vowels = set('AEIOU')
    new_letters = [letter for letter in word if letter.upper() not in vowels]
    print(''.join(new_letters))

… but the basic idea is the same.

abarnert
  • 354,177
  • 51
  • 601
  • 671
  • 1
    `if letter.upper() not in set('AEIOU')`: let me tell you that it's not very efficient because `set('AEIOU')` is built at every iteration. So much for a speed gain. Either build the set beforehand, or use `{'A','E','I','O','U'}` so it's literal. Besides, when there are 5 letters, using a `set` doesn't help much. – Jean-François Fabre Mar 18 '18 at 08:44
  • @Jean-FrançoisFabre Good point. And it's probably clearer to the OP to name the set anyway. (I was using a set more because conceptually we're checking if it's in a set of letters, not so much for efficiency, but your change gives both benefits.) – abarnert Mar 18 '18 at 09:02
  • it's already much better. In a real program, I would've make it global so it's only built once (or use the literal form). But that's just nitpicking. – Jean-François Fabre Mar 18 '18 at 09:07
  • @Jean-FrançoisFabre Of course if you're really worried about performance, that `LOAD_GLOBAL` each time through the loop could make a difference; you might want to assign it to a local (explicitly at the top of the function, or via a default parameter value). But it's rarely going to matter. – abarnert Mar 18 '18 at 09:09
  • you may have noticed that I'm a performance maniac :) the issue is that some people take examples here literally and adapt the code to their very large dataset. Then they complain: your answer isn't working :) – Jean-François Fabre Mar 18 '18 at 09:59
  • 1
    @Jean-FrançoisFabre I care more about people learning than people getting code they can use without thinking, and I don't really care if they complain later that they couldn't use it without thinking. Besides, I've never see anyone complain that the code is 5% too slow. Usually when they're saying "What's the most efficient way to do this", it's not because they need to micro-optimize, but because they used an exponential-time recursive algorithm for no good reason. – abarnert Mar 18 '18 at 10:02
  • @abarnert Thanks a lot. I see people just finding an alternative solution rather than clarifying on things. People like you make this community an awesome place to be. Good day! – Anush Poudel Mar 18 '18 at 12:44
-2

This should help.

def disemvowel(word):
    words = list(word)
    v = ["a", "e", "i", "o", "u"]     #list of vowel 
    return "".join([i for i in words if i.lower() not in v])  #check if alphabet not in vowel list 
print disemvowel("uURII")

Output:

R
Rakesh
  • 81,458
  • 17
  • 76
  • 113
  • 1
    If you're just going to give him new code without explaining what it does, or what's wrong with his existing code, at least make it simple and idiomatic. Why are you doing `words = list(word)` here? Why is `v` a list instead of a set (or maybe a string)? And meanwhile, using `lower` isn't wrong here, but it's going to make the OP think there's something wrong with the way he used `upper` in his attempt, when there isn't. – abarnert Mar 18 '18 at 08:36