3
\$\begingroup\$

Converting words to morse code and vice versa using dictionaries and that's it.

morse_code = {'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': '--..', '1': '.----', '2': '..---', '3': '...--', '4': '....-', '5': '.....',
              '6': '-....', '7': '--...', '8': '---..', '9': '----.', '0': '-----', ' ': '   '}


def convert_to_morse_code(word):
    string = ''

    for letternum in word:
        if letternum.isalnum():
            string += f'{morse_code[letternum.upper()]} '
        else:
            string += '  '

    return string


def convert_to_word(word):
    string = ''

    if '   ' in word:
        word = word.replace('   ', '</<')
    if ' ' in word:
        word = word.replace(' ', '<')
    for code in word:
        if code == '/':
            word = word.replace(code, '   ')

    word = word.split('<')

    for code in word:
        for key in morse_code.keys():
            if morse_code[key] == code:
                string += key

    return string


print(convert_to_morse_code('HI MICHAEL'))
print(convert_to_word('.... ..   -- .. -.-. .... .- . .-..'))
\$\endgroup\$
  • 2
    \$\begingroup\$ Do you want any specific aspects of your code reviewd? \$\endgroup\$ – πάντα ῥεῖ Aug 28 '19 at 17:08
3
\$\begingroup\$

The question could benefit from some context. You don't need to answer every one of the following, but it would be helpful for reviewers.

  1. Why have you written a morse code converter?
  2. Is this going to be used by anyone else or is it a personal project? Is it a homework question? From this we can tell if the review should focus on finding security holes, or just annoying bugs.
  3. What will the input look like? Can we assume nobody is going to call convert_to_word with a letters that aren't dot, dash, or space?

def convert_to_morse_code(word):
    ...

def convert_to_word(word):
    ...

This looks a little funny, as they both take a "word", yet neither does. One takes a string which may have complete sentences, and the other takes a string containing morse code. I'd suggest adding a docstring for these functions which gives a brief explanation about the expected input.


if letternum.isalnum():
    string += f'{morse_code[letternum.upper()]} '
else:
    string += '  '

What happens if somebody comes along and helpfully adds some international morse code symbols such as "?" or "+" to the dictionary? This code will ignore those. I think the dictionary should be the source of truth, not isalnum.

if letternum.upper() in morse_code:
    string += morse_code[letternum.upper()] + ' '
else:
    string += '  '

The above possibility shows that letternum is no longer an appropriate name. Lets update it the name to c or char, short for character, which is what it is. The other change I would propose here is to use the dictionary method .get as it has a handy parameter 'default value'. If we set the default value as ' ' (for when the character is not a symbol we know how to write in morse code) this takes the place of the else branch.

for char in word:
    string += morse_code.get(char, ' ') + ' '

I think it would be worth noting in the docstring for this function that unrecognised characters are skipped, and this is the line responsible.


for code in word:
    if code == '/':
        word = word.replace(code, '   ')

In general it is a bad idea to modify the thing you are looping over. It can lead to hard to spot bugs. The other thing to note is that the first time '/' is encountered, all occurrences of it are replaced. This doesn't need to be in a loop.


    for key in morse_code.keys():
        if morse_code[key] == code:
            string += key

This loop does a little more work than is necessary. It will get slower and slower each time a new symbol is added to the dictionary. The performance loss will honestly not be noticeable, but if you think the solution sounds nicer you can try implementing it.

The solution is to build a new map which is the inverse of morse_code, where all the keys become values and all the values become keys.

morse_code = {...}
code_to_char = {code: char for char, code in morse_code.items()}
...

    if code in code_to_char:
        string += code_to_char[code]

There are some small problems to which you will need to decide on the answer.

  1. convert_to_word(convert_to_morse_code("Hello, world!")) outputs 'HELLO WORLD '. Is that ok? Should the output match the input more closely? Why is there trailing space?
  2. convert_to_morse_code("HI LOW") outputs '.... .. .-.. --- .-- '. Some variations exist, such as using a slash to indicate a space ('.... .. / .-.. --- .--') or seven dots (.... .. ....... .-.. --- .--). Could your code have that as an optional feature?
  3. convert_to_word works on some weird input, and not on others. But it does not ever tell the user when something is wrong. I'd prefer if convert_to_word('--- -, .') failed in some way rather than returning 'OE'.
\$\endgroup\$
1
\$\begingroup\$
if __name__ == '__main__':

use this guard that allows you to import from this script without running the code.

if __name__ == '__main__':
    print(convert_to_morse_code('HI MICHAEL'))
    print(convert_to_word('.... ..   -- .. -.-. .... .- . .-..'))

Style

check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide and here are a few comments:

  • Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. An object's docstring is defined by including a string constant as the first statement in the object's definition. Use docstrings in the following way to indicate what your functions do:

    def convert_to_morse(text):
        """Convert text to Morse code."""
        # code goes here
    
    def convert_morse_to_word(morse_code):
        """Convert Morse code to alphanumeric words."""
        # code goes here
    
  • Blank lines use blank lines sparingly (too many blank lines in your script)

    string = ''
    
    if '   ' in word:
        word = word.replace('   ', '</<')
    if ' ' in word:
        word = word.replace(' ', '<')
    for code in word:
        if code == '/':
            word = word.replace(code, '   ')
    
    word = word.split('<')
    
    for code in word:
        for key in morse_code.keys():
            if morse_code[key] == code:
                string += key
    
    return string
    

Code

  • f- strings provide a way to embed expressions inside string literals, using a minimal syntax and are not supposed to be used this way:

    string += f'{morse_code[letternum.upper()]} '
    

    could be written:

    string += morse_code[letternum.upper()]
    
  • isalnum() a bad idea if you want to include punctuation and spaces and Morse code does not exclude punctuation btw.

  • string += this is inefficient because strings cannot be changed in place so each time you add to the string, a new string is created. A better approach is to use list comprehensions and join the results. We have to reconstruct dictionary based on the following international Morse code:

International Morse code.

and the code looks like:

def get_translation(translate_to):
    """
    Return a dictionary from to (Morse code-text)
    assuming translate_to a string:
    m for translation to Morse
    t for translation to text.
    """
    morse_code = {
        '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': '--..',
        'á': '.--.-', 'ä': '.-.-', 'é': '..-..', 'ñ': '--.--', 'ö': '---.', 'ü': '..--', "'": '·----·',
        '1': '.----', '2': '..---', '3': '...--', '4': '....-', '5': '.....',
        '6': '-....', '7': '--...', '8': '---..', '9': '----.', '0': '-----', '!': '-·-·--',
        '.': '.-.-.-', ',': '--..--', '?': '..--..', ':': '---...', "\"": '.-..-.',
        '-': '-....-', '/': '-..-.', '(': '-.--.', ')': '-.--.-', ' ': '\t', '\n': '\t', '_': '··--·-'
    }
    morse_to_letter = {code: letter for letter, code in morse_code.items()}
    if translate_to == 'm':
        return morse_code
    if translate_to == 't':
        return morse_to_letter
    else:
        raise ValueError(f'Invalid input{translate_to} expected m or t')


def convert_to_morse_code(text):
    """Translate text to Morse code."""
    morse_code = get_translation('m')
    return ' '.join([morse_code[letter] for letter in text.lower()])


def convert_morse_code_to_text(morse_code):
    """Translate from Morse code to text."""
    morse_to_letter = get_translation('t')
    text = []
    words = morse_code.split('\t')
    for word in words:
        letters = word.split()
        to_text = [morse_to_letter[letter] for letter in letters]
        text.append(''.join(to_text))
    return ' '.join(text)


if __name__ == '__main__':
    print(convert_to_morse_code('Hello Michael! How are you doing today?'))
    print(convert_morse_code_to_text('·· ·----· --   ..-. ·· -· .    - ···· ·- -· -.- ... .-.-.-'))
\$\endgroup\$
  • \$\begingroup\$ I am not the most familiar with the single-line list comprehensions could you dissect the code a little more, please? Additionally, I derived this from CodeWars where spaces between words are 3, and for letters, are 1. \$\endgroup\$ – miAK Aug 28 '19 at 21:30
  • \$\begingroup\$ Might want to check this link: programiz.com/python-programming/list-comprehension \$\endgroup\$ – user203258 Aug 28 '19 at 21:30
  • \$\begingroup\$ You should've indicated the 3 spaces thing in the description above and a link to the problem indicating input and desired output if you want the results to be exact. However the code I provided does the same functionality using a tab for words '\t' and 1 space for letters. \$\endgroup\$ – user203258 Aug 28 '19 at 21:33

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.