I am a relatively new Python programmer and made a simple Pig Latin to English translator and vice versa. I would just like it if someone reviewed my code and see if anything can be done to make it more Pythonic or efficient.
from time import sleep
def main():
to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ")
while to_piglatin not in("p", "e"):
to_piglatin = input("[P/E]")
to_piglatin = ("e", "p").index(to_piglatin.lower())
if to_piglatin:
lang = "pig latin"
else:
lang = "english"
question = "Enter a sentence to translate it into {}: ".format(lang)
sentence = input(question)
if not sentence.isalpha():
print("Invalid input, please do not use numbers or punctuation marks")
sleep(1)
main()
split_sentence = sentence.split()
if to_piglatin:
translated_sentence = [translate_to_piglatin(word) for word in split_sentence]
else:
translated_sentence = [translate_to_english(word) for word in split_sentence]
translated_sentence = " ".join(translated_sentence)
print(translated_sentence)
input()
return True
def translate_to_piglatin(word):
vowels = 'aeiou'
list_word = list(word)
list_word = [x.lower() for x in word]
first_letter = list_word[0]
last_letter = list_word[-1]
if last_letter.isalpha() == False:
list_word.remove(last_letter)
if first_letter in vowels:
list_word.append('y')
list_word.append('a')
list_word.append('y')
else:
list_word.remove(list_word[0])
list_word.append(first_letter)
list_word.append('a')
list_word.append('y')
translated_word = "".join(list_word)
return translated_word
def translate_to_english(word):
list_word = list(word)
last_three_letters = list_word[-3:]
last_three_letters = "".join(last_three_letters)
if last_three_letters == 'yay':
vowel = True
else:
vowel = False
if vowel:
for i in range(0,2):
del list_word[-1]
else:
del list_word[-1]
del list_word[-1]
last_letter = list_word[-1]
list_word.remove(last_letter)
list_word.insert(0, last_letter)
translated_word = "".join(list_word)
return translated_word
main()
2 Answers 2
There is this thing in Python called unpacking.
The idea being that you can assign at once in several variables all the values contained in a single sequence:
>>> example = (1, 2, 3)
>>> a, b, c = example
>>> print(a, b, c)
1 2 3
In fact, this mechanism works with any iterable, including strings. What's even better is that this syntax got extended in Python 3 to support assigning the remaining of the items into a single variable:
>>> a, *b, c = range(5)
>>> print(a, c)
0 4
>>> print(b)
[1, 2, 3]
Now, let's make good use of it:
def translate_to_piglatin(word):
vowels = 'aeiou'
first_letter, *remaining_letters = word.lower()
if first_letter in vowels:
return word + 'yay'
else:
return ''.join(remaining_letters) + first_letter + 'ay'
Here I removed the need for first_letter, *inner_part, last_letter = word.lower()
because I assumed that word
was not ill-formed. Since you check for isalpha
in your main
, you don't really need to do so in this function.
An other improvement would be to define vowel
as a constant outside of this function.
Converting back can also easily make use of unpacking:
def translate_to_english(word):
if word.endswith('yay'):
return word[:-3]
else:
*remaining_letters, last_letter, a, y = word
return last_letter + ''.join(remaining_letters)
In this case:
- I used a condition to enter the
if
rather than setting a boolean before; - I used the slice syntax to extract out the last 3 characters directly.
In both cases I returned from within the if
to simplify the logic and the variable management. Each branch needing its own kind of data structure to work with.
The last thing unpacking can help you with is the choice of language/transformation function. If you define, for each input letter, the name of the language and the associated transformation function in a tuple, you can extract them out easily:
CONVERSION_INFOS = {
'e': ('english', translate_to_english),
'p': ('pig latin', translate_to_piglatin),
}
def main():
to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ").lower()
while to_piglatin not in CONVERSION_INFOS:
to_piglatin = input("[P/E]").lower()
lang, translate = CONVERSION_INFOS[to_piglatin]
sentence = input("Enter a sentence to translate it into {}: ".format(lang))
if not sentence.isalpha():
print("Invalid input, please do not use numbers or punctuation marks")
sleep(1)
main()
translated_sentence = ' '.join(translate(word) for word in sentence.split())
print(translated_sentence)
input()
return True
Note the use of lower
on the first two input
s so your while
loop can break if the user input 'E'
or 'P'
.
I also merged some lines together to remove the intermediate variables you only use once and reduce the burden of understanding your code for the reader.
Lastly, you can improve the workflow of your program by making a correct use of return values. You don't need to return something at the end of a function in Python (meaning return True
in main
is unneccessary) when there is no associated meaning. However, we can make use of it:
- We return
True
if we correctly translated a sentence; - We return
False
if we couldn't.
def main():
to_piglatin = input("Do you want to translate something to piglatin[P], or to english[E]: ").lower()
while to_piglatin not in CONVERSION_INFOS:
to_piglatin = input("[P/E]").lower()
lang, translate = CONVERSION_INFOS[to_piglatin]
sentence = input("Enter a sentence to translate it into {}: ".format(lang))
if not sentence.isalpha():
print("Invalid input, please do not use numbers or punctuation marks")
return False
translated_sentence = ' '.join(translate(word) for word in sentence.split())
print(translated_sentence)
return True
This let you handle retries outside of the function:
while not main():
pass # or time.sleep(1) like you did, but it feels a bit unresponsive
You may also want to change the name of the function because it doesn't convey much meaning when calling it like that.
-
\$\begingroup\$ You've got my +1 for the first explanation. Nicely done \$\endgroup\$Grajdeanu Alex– Grajdeanu Alex2016年05月09日 09:26:02 +00:00Commented May 9, 2016 at 9:26
Variables and Naming
to_piglatin
isn't a very good name. Try making itlanguage
Speaking of
lang
, it doesn't look like you need it.You only use question once. Just put it inside the call to input on the next line.
Change
list_word
toword_list
. That sounds more natural.The main method should be in something like this: (more pythonic)
if __name__ == "__main__": main()
That is the standard for main methods, so if you, at a later date, decide to import you code from another file, the main method will not run and you can still use the translation functions.
Code logic
- Repeated calls to main can eventually overflow the stack trace (heh, stackoverflow). Try making a Boolean that keeps track of the user has inputed a correct value, and put the entire input sequence in a while loop.
- Why do you have an empty input?
- Why does the main method return
True
? If anything (which it shouldn't, having a name likemain
), it should return the translated list. "the"
gets translated to"hetay"
(not actual piglatin)- You don't have to enter an if statement to set a Boolean variable; just set it to the thing in the if.
That's about it for me. Other people will probably have better suggestions. Happy coding!
-
\$\begingroup\$ You can get code blocks inside bullet points by indenting a lot. It will look better and preserve newlines and indentation. \$\endgroup\$anon– anon2016年05月09日 03:52:31 +00:00Commented May 9, 2016 at 3:52
-
\$\begingroup\$ @MathiasEttinger well, I was stupid. I guess I skipped a line. \$\endgroup\$Blue– Blue2016年05月09日 10:02:43 +00:00Commented May 9, 2016 at 10:02
Explore related questions
See similar questions with these tags.