9
\$\begingroup\$

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()
\$\endgroup\$
11
\$\begingroup\$

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 inputs 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.

\$\endgroup\$
  • \$\begingroup\$ You've got my +1 for the first explanation. Nicely done \$\endgroup\$ – Grajdeanu Alex. May 9 '16 at 9:26
6
\$\begingroup\$

Variables and Naming

  1. to_piglatin isn't a very good name. Try making it language
  2. Speaking of lang, it doesn't look like you need it.
  3. You only use question once. Just put it inside the call to input on the next line.
  4. Change list_word to word_list. That sounds more natural.
  5. 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

  1. 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.
  2. Why do you have an empty input?
  3. Why does the main method return True? If anything (which it shouldn't, having a name like main), it should return the translated list.
  4. "the" gets translated to "hetay" (not actual piglatin)
  5. 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!

\$\endgroup\$
  • \$\begingroup\$ You can get code blocks inside bullet points by indenting a lot. It will look better and preserve newlines and indentation. \$\endgroup\$ – Fund Monica's Lawsuit May 9 '16 at 3:52
  • \$\begingroup\$ @MathiasEttinger well, I was stupid. I guess I skipped a line. \$\endgroup\$ – Blue May 9 '16 at 10:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.