On Wed, Oct 28, 2015 at 10:09 AM, Vusa Moyo <sow...@gmail.com> wrote: > Hi Guys, > > I've written a script to remove vowels from a string/sentence. > > the while loop I'm using below is to take care of duplicate vowels found in > a sentence, ie > > anti_vowel('The cow moos louder than the frog') > > It works, but obviously its messy and n00by. Any suggestions on how I can > write this code better?
First off, the code that works (by "works" I mean "passes its tests") is far better than the code that doesn't, no matter what form that code takes. That said, there are several tricks you can use to significantly shorten your function here. > def anti_vowel(text): I'd rather name this "remove_vowels", since that's what it *does* rather than what it *is* is an English sense. > vowel = ['a', 'e', 'i', 'o', 'u'] > VOWEL = ['A', 'E', 'I', 'O', 'U'] Strings can be thought of as tuples optimized for characters, with some special methods. Tuples, in turn, can be thought of as immutable lists. So the above can be shortened to: vowel = 'aeiou' VOWEL = 'AEIOU' Using one of those special methods I mentioned: vowel = 'aeiou' VOWEL = vowel.upper() But do we really need two separate vowel containers? vowels = 'aeiou' all_vowels = vowels + vowels.upper() Or to save a name, and some typing: vowels = 'aeiou' vowels += vowels.upper() > manip = [] > > for i in text: > manip.append(i) Any time you have the pattern "create an empty list, then append to it in a for loop", think "comprehension". The above could be shortened to: manip = [i for i in text] Or, since the list constructor takes an iterable argument and strings give characters upon iteration: manip = list(text) But we don't really need manip at all, as I'll show further down. > fufu = 0 > # > while fufu < 16: This is a fairly weird way to spell "loop 8 times", if I'm honest :). A more idiomatic approach would be to get rid of 'fufu' and do this instead: for each in range(8): Which loops 8 times and assigns the number of the loop (0-7) to the name 'each' each time around. 'each' could be any valid identifier ('_' is commonly used for this), 'each' just makes sense reading the line as English. > for x in vowel: > if x in manip: > manip.remove(x) > > for y in VOWEL: > if y in manip: > manip.remove(y) Above, we combined 'vowel' and 'VOWEL' into 'vowels', so these can be shortened into a single loop by removing the second loop and changing the first to iterate over 'vowels' instead of 'vowel'. But instead of removing values from a list we just built, it's easier to build the list without those values in the first place, see below. > fufu = fufu + 2 This line goes away with the change from 'while' to 'for each in range'. > strong = ''.join(manip) > return strong I suspect this was meant to be 'string' :). Anyway, 'return' can return any expression not just a name, so this could be just: return ''.join(manip) So, what was I talking about with not needing 'manip' and building the list without the values in the first place? Combining some other stuff mentioned above, we can build a list of the characters of the given text, minus vowels, using a comprehension: list_of_chars_without_vowels = [c for c in text if c not in vowels] To better understand what's going on here, you can "unroll" the comprehension by initializing the name to an empty list, then moving the initial expression (the "c" in "c for ...") to an append call all the way inside: list_of_chars_without_vowels = [] for c in text: if c not in vowels: list_of_chars_without_vowels.append(c) 'list_of_chars_without_vowels' is then in the same state your 'manip' was in after the loops, so the loops go away entirely. 'list_of_chars_without_vowels' is an unwieldy name, but we don't actually need to name it: return ''.join([c for c in text if c not in vowels]) And one final optimization (which in this case is more of a finger optimization than a performance optimization), we can drop the square brackets to turn the list comprehension into a generator expression: return ''.join(c for c in text if c not in vowels) The difference between the two above statements is that the first calculates the entire list of characters, then passes it to ''.join(), which iterates through the list to create the final string, while the second creates a generator and passes it to ''.join(), and the generator calculates and yields the next character each time ''.join() calls next() on it. To put everything together, here's my final version, with a docstring, a punny Easter egg, and a very simplistic test that is not adequate testing: import random def remove_vowels(text): """Remove all vowels from 'text' and return the result.""" vowels = 'aeiou' + random.choice(['y', '']) # "and sometimes y" vowels += vowels.upper() return ''.join(c for c in text if c not in vowels assert remove_vowels('Did It work? Looks like.') == 'Dd t wrk? Lks Lke.' Hoping I may have taught you something, -- Zach _______________________________________________ Tutor maillist - Tutor@python.org To unsubscribe or change subscription options: https://mail.python.org/mailman/listinfo/tutor