10
\$\begingroup\$

I am a beginner in Python, and I made a Morse translator that converts letters to Morse, and Morse to letters (where a decimal point is 'dot' and underscore is 'dash'). Is there any way I could make my code shorter and more efficient? For reference, here's the translation of Morse to letter characters.

dot = '.'
dash = '_'
letter_list = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S',
               'T', 'U', 'V', 'W', 'X', 'Y', 'Z']
morse_list = [dot + dash, dash + dot * 3, dash + dot + dash + dot, dash + dot * 2, dot, dot * 2 + dash + dot,
              dash * 2 + dot, dot * 4, dot * 2, dot + dash * 3, dash + dot + dash, dot + dash + dot * 2,
              dash * 2, dash + dot, dash * 3, dot + dash * 2 + dot, dash * 2 + dot + dash, dot + dash + dot,
              dot * 3, dash, dot * 2 + dash, dash * 3 + dot, dot + dash * 2, dash + dot * 2 + dash,
              dash + dot + dash * 2, dash * 2 + dot * 2]
# The for loop below prints the entire translation (Eg. 'A: ._ B: _...') so the user could reference it
for n in range(len(letter_list)):
    print(letter_list[n] + ': ' + morse_list[n], sep=' ', end=' ', flush=True)
while True:
    print("\nTYPE 'EXIT' if you want to EXIT.")
    user_input = input("INPUT LETTERS OR MORSE CHARACTERS (DECIMAL AND UNDERSCORE) TO CONVERT."
                       "\nSEPERATE LETTERS WITH SPACES: ")
    # Converts user input into an iterable list
    user_list = user_input.split()
    if user_input == 'EXIT':
        print("THANK YOU FOR USING THE MORSE TRANSLATOR")
        break
    for i in range(len(user_list)):
        if user_list[i] in letter_list:
            for n in range(len(morse_list)):
                if user_list[i] == letter_list[n]:
                    print(morse_list[n], sep=' ', end='  ', flush=True)
        elif user_list[i] in morse_list:
            for n in range(len(letter_list)):
                if user_list[i] == morse_list[n]:
                    print(letter_list[n], sep=' ', end='  ', flush=True)
\$\endgroup\$
5
  • \$\begingroup\$ Yes. Mixed translations are allowed. So if the user inputs 'A ._' it will output, '._ A' \$\endgroup\$ – Star Man Dec 10 '20 at 18:02
  • \$\begingroup\$ I would suggest a dictionary \$\endgroup\$ – theProgrammer Dec 10 '20 at 18:24
  • 1
    \$\begingroup\$ Since the morse code are known at compile time, why calculate them? You should simply map A: '.-' instead, this saves the multiplication and addition cost \$\endgroup\$ – theProgrammer Dec 10 '20 at 18:30
  • \$\begingroup\$ @theProgrammer Thank you for the tips. The reason I did not map it was because I might want to change the value of dot and dash. So dot could be either • and — rather than . and _. If it's in a variable, i would only have to change 2 lines, rather than an entire dictionary. \$\endgroup\$ – Star Man Dec 10 '20 at 23:28
  • \$\begingroup\$ Concatenation of strings are relatively expensive but its okay for this scenario, but do keep that in mind. \$\endgroup\$ – theProgrammer Dec 11 '20 at 12:53
12
\$\begingroup\$
  1. Since it is really difficult to tell what letter maps to what, it will be much easier to validate your code (for yourself & future maintainers) if you write out the dictionary directly:
CHAR_TO_MORSE = {
    'A': '._',
    'B': '_...',
    'C': '_._.',
    'D': '_..',
    'E': '.',
    'F': '.._.',
    'G': '__.',
    'H': '....',
    'I': '..',
    'J': '.___',
    'K': '_._',
    'L': '._..',
    'M': '__',
    'N': '_.',
    'O': '___',
    'P': '.__.',
    'Q': '__._',
    'R': '._.',
    'S': '...',
    'T': '_',
    'U': '.._',
    'V': '___.',
    'W': '.__',
    'X': '_.._',
    'Y': '_.__',
    'Z': '__..',
}
MORSE_TO_CHAR = {v: k for k, v in CHAR_TO_MORSE.items()}
CHAR_OR_MORSE_TO_INVERSE = {**CHAR_TO_MORSE, **MORSE_TO_CHAR}
  1. Avoid string concatenation (+) when possible (mostly for performance):
print(' '.join("{}: {}".format(k, v) for k, v in CHAR_TO_MORSE.items()))
  1. Very minor but check exit condition before parsing:
if user_input == 'EXIT':
    break
user_list = user_input.split()
  1. What happens if the input contains neither a single char nor valid morse? Your program will eat it, potentially at the confusion of the user. Maybe you can default to printing a question mark; something like:
print(' '.join(CHAR_OR_MORSE_TO_INVERSE.get(c, '?') for c in user_list))

Note that this uses a single dictionary lookup per item user_list, which is about as efficient as you could ask for.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ +1 for the nice review. There is an extra { after CHAR_TO_MORSE and a missing ) in the first print. The first print can also be shorten using f-strings: print(' '.join(f'{k}: {v}' for k, v in CHAR_TO_MORSE.items())). \$\endgroup\$ – Marc Dec 11 '20 at 2:17
4
\$\begingroup\$

There is no need to store characters in a list when you can do it in a string.

Instead of the confusion index checks all over your code, use can usea 2 python dict:

dot = '.'
dash = '_'
letter_list = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
morse_list = [dot + dash,
              dash + dot * 3,
              dash + dot + dash + dot,
              dash + dot * 2, dot,
              dot * 2 + dash + dot,
              dash * 2 + dot,
              dot * 4, dot * 2,
              dot + dash * 3,
              dash + dot + dash,
              dot + dash + dot * 2,
              dash * 2,
              dash + dot, dash * 3,
              dot + dash * 2 + dot,
              dash * 2 + dot + dash,
              dot + dash + dot,
              dot * 3, dash,
              dot * 2 + dash,
              dash * 3 + dot,
              dot + dash * 2,
              dash + dot * 2 + dash,
              dash + dot + dash * 2,
              dash * 2 + dot * 2]

dct = dict(list(zip(letter_list, morse_list)) + list(zip(morse_list, letter_list)))

print(" ".join(f"{k}: {dct[k]}" for k in dct))

while True:
    print("\nTYPE 'EXIT' if you want to EXIT.")
    user_input = input("INPUT LETTERS OR MORSE CHARACTERS (DECIMAL AND UNDERSCORE) TO CONVERT."
                       "\nSEPERATE LETTERS WITH SPACES: ")
    if user_input == 'EXIT':
        print("THANK YOU FOR USING THE MORSE TRANSLATOR")
        break
    user_list = user_input.split()
    print(" ".join(dct.get(i) for i in user_list))

For more understanding of what dict(list(zip(letter_list, morse_list)) + list(zip(morse_list, letter_list))) does:

  • list(zip([1, 2, 3], "abc")) would return [(1, 'a'), (2, 'b'), (3, 'c')]
  • list(zip("abc", [1, 2, 3])) would return [('a', 1), ('b', 2), ('c', 3)]
  • list(zip([1, 2, 3], "abc")) + list(zip("abc", [1, 2, 3])) would return [(1, 'a'), (2, 'b'), (3, 'c'), ('a', 1), ('b', 2), ('c', 3)]
  • dict(list(zip([1, 2, 3], "abc")) + list(zip("abc", [1, 2, 3]))) would return {1: 'a', 2: 'b', 3: 'c', 'a': 1, 'b': 2, 'c': 3}

UPDATE:

d = dict(zip(letter_list, morse_list))
d.update(zip(morse_list, letter_list))

is more efficient than

d = dict(list(zip(letter_list, morse_list)) + list(zip(morse_list, letter_list)))

Credits to GZ0 and superb rain in the comments.

\$\endgroup\$
0

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.