0
\$\begingroup\$

I have a code for a short Python 2 Pig Latin translator. Is it a good code, or should it be revised?

pyg = 'ay'
original = raw_input('Enter a word:')
# allows user input ^

if len(original) > 0 and original.isalpha():
word = original.lower()
first = word[0]
new_word = word + first + pyg
new_word = new_word[1:len(new_word)]
else:
print 'empty'

# prints the inputted word. If someone typed 'Hello', it will translate into Pig Latin.
print new_word

It works, but a shorter code may or may not be a good thing.

\$\endgroup\$
  • 7
    \$\begingroup\$ Can you please fix the indentation? I can guess what you're intending, but really code needs to be valid to be ontopic here. \$\endgroup\$ – Carcigenicate Nov 6 '19 at 22:02
  • \$\begingroup\$ Welcome to Code Review! Currently your code doesn't perform because of Python's need for code blocks to be indented, please fix the indentation and then we can reopen this question. I assume that indenting will not invalidate the answer already given. \$\endgroup\$ – Malachi Nov 8 '19 at 14:56
3
\$\begingroup\$

First, note how you're doing a if len(original) > 0 and original.isalpha(): check, then printing "empty" if it fails. This means though that "empty" will be printed out if you input a non-alphabetical word, which is confusing. I'd either handle those two checks separately, or print out a generic error message instead like "Invalid Input".


You have

new_word[1:len(new_word)]

To discard the first letter. There's actually a short-cut for this though:

new_word[1:]

If you omit the second argument to the slice operator, it defaults to the length of the collection. If you omit the first, it defaults to the beginning of the collection. Combined, that's why my_list[:] makes a shallow copy of my_list.


first = word[0]
new_word = word + first + pyg
new_word = new_word[1:]

This chunk breaks off the first letter, does some concatenation, then chops off the first character of the new string. Note though that word will be on the front of new_word, so you can just chop off the first character of word at the same time you do word[0].

If you were using Python 3, you could simply write:

first, *rest_word = word
new_word = ''.join(rest_word) + first + pyg

It's just deconstructing the string, except that instead of the second character being stored in rest_word, everything that's left is stored in it. This essentially just separates the "head" of the string from the "tail".

Unfortunately, I came back a few hours later and realized that my original code that I had posted was wrong. rest_word is a list of strings, not a string itself. This necessitated the ''.join(rest_word) bit. This method isn't quite as efficient in this case, but you may find that it's useful if you don't care what exact type the rest * variable is. If you just want to iterate it, it wouldn't matter.


You overwrite new_word instead of creating a new variable. I would have created a new one to preserve the old new_word. That can be helpful when debugging.


On success, you print out new_word outside of the if. That makes less sense than printing inside of it, and it causes an error to be raised if the if was False since new_word isn't defined.


At the top you have pyg. It's a constant though, and according to Python's style guide, constants should be uppercase, and separated by underscores. I'd also make it more descriptive:

PIG_LATIN_SUFFIX = 'ay'


You stated that the goal was to be short, so I won't go too into this, but you should make use of functions here. You have a couple discreet things going on here:

  • Handling input from the user
  • Doing the conversion on the input

And you have both of those things mixed up together.

I'd separate things out properly, and do some basic input checking:

PIG_LATIN_SUFFIX = 'ay'


def convert_to_pig_latin(original_word):
    word = original_word.lower()
    first = word[0]
    rest_word = word[1:]

    return rest_word + first + PIG_LATIN_SUFFIX


def ask_for_input():
    while True:
        word = raw_input('Enter a word:')

        if len(word) > 0 and word.isalpha():
            return word

        else:
            print("Invalid Input. Please try again.")


def main():
    original_word = ask_for_input()
    pig_word = convert_to_pig_latin(original_word)
    print pig_word

ask_for_input will ask until it gets valid input now instead of just ending the program.



Oh ya, and switch to Python 3! Python 2 is about to have support for it discontinued.

\$\endgroup\$
  • \$\begingroup\$ Well, im going to try that. \$\endgroup\$ – Hunter Nov 8 '19 at 18:34

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