9
\$\begingroup\$

I've posted the code for the following question. What could be improved?

Q: Write a program that allows the user to type in a phrase and then outputs the acronym for that phrase.

Note: The acronym should be all uppercase, even if the words in the phrase are not capitalized.

def main():

    ## input the phrase
    phrase = input("Enter the phrase: ")

    ## split the phrase into substrings
    phrase_split = phrase.split()


    acronym = ""

    ## iterate through every substring
    for i in phrase_split:
        acronym = acronym + i[0].upper()

    print("The acronym for your phrase is ",acronym + ".")

main()
\$\endgroup\$
  • \$\begingroup\$ What would be – and what should be – an acronym for a phrase with part in parentheses (or in dashes), like this one? \$\endgroup\$ – CiaPan Jun 26 '18 at 16:13
  • \$\begingroup\$ Is this aimed at being a whiteboard exercise? \$\endgroup\$ – chicks Jul 2 '18 at 18:48
8
\$\begingroup\$
  1. You should name your function more descriptively, maybe acronym.
  2. You should wrap your code under a if __name__ == "__main__": guard to allow importing parts of your script from other scripts.
  3. You should separate the user input from the logic.
  4. Use docstrings to describe what functions do.
  5. Use str.join together with a generator expression to join multiple strings.
  6. Use the new f-strings introduced in Python 3.6.

Taking this gives you:

def user_input():
    """Get the user input for a phrase"""
    return input("Enter the phrase: ")

def acronym(words):
    """Turn a list of words into an acronym"""
    return "".join(word[0] for word in words).upper()

def main():
    words = user_input().split()
    print(f"The acronym for your phrase is {acronym(words)}.")

if __name__ == "__main__":
    main()
\$\endgroup\$
  • \$\begingroup\$ You've not said but you did apply to your code: "Call upper only once instead of once per word". \$\endgroup\$ – SylvainD Jun 26 '18 at 16:10
  • \$\begingroup\$ Function (user_input) should has a verb in its name, because functions do something. Also, I would not have separate function for getting user input, because this is impure part of program and I think appropriate place for that is function main(). I would have functions for pure part and main() for impure one. Pure part: from input string to acronym. Impure part: getting input from user and printing result \$\endgroup\$ – Stexxe Jun 26 '18 at 20:04
2
\$\begingroup\$

I would rather write the code as soft as the requested exercise was:

def acronym(phrases):
    acronym = ""
    for word in phrases.split():
        acronym = acronym + word[0].upper()
    return acronym

def main():
    phrases = input("Enter a phrase: ")
    print(acronym(phrases))

if __name__ == "__main__":
    main()

I changed some variable names to make everything more readable and removed something useless to shorten the code.

You should wrap your code under a if __name__ == "__main__": guard to allow importing parts of your script from other scripts.

as the other guy said.

I can bear that you are a novice and the first times what matters is to make a good and clear code, so you can get the idea next times you have to do something similar.

If it was asked to make the code the shortest possible, then I would suggest something like this:

def main():
    data = input('Enter a phrase: ')
    data = [i[0] for i in data.upper().split(' ')] 
    print(''.join(data))

if __name__ == "__main__":
    main()

Here we split every word, get its first letter and use str.join() to print a concatenation of the strings.

\$\endgroup\$

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.