3
\$\begingroup\$

I created this morse code encoder and decoder.

class DecodeError(BaseException):
    __module__ = Exception.__module__

class EncodeError(BaseException):
    __module__ = Exception.__module__


code_letter = {
    '.-': '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',

    '--..--': ', ', '.-.-.-': '.', '..--..': '?', '-..-.': '/',
    '-....-': '-', '-.--.': '(', '-.--.-': ')', '/': ' '
}


letter_code = {value: key for key, value in zip(code_letter.keys(), code_letter.values())}


def morse_encode(string: str) -> str:
    try:
        return ' '.join(letter_code[i.upper()] for i in string)
    except:
        raise EncodeError('Unknown value in string')


def morse_decode(string: str) -> str:
    try:
        return ''.join(code_letter[i] for i in string.split())
    except:
        raise DecodeError('Unknown value in string')

if __name__ == '__main__':
    string = input()
    print(morse_encode(string))

Is it possible to make the code shorter while maintaining neatness?

Thanks!

\$\endgroup\$
4
\$\begingroup\$

@l0b0 is right for (1.). The docs say to inherit from Exception instead:

The base class for all built-in exceptions. It is not meant to be directly inherited by user-defined classes (for that, use Exception)

I've also never seen __module__ used in that case, but if you like to output better, I guess it works fine.


letter_code can make use of dictionary's items method.

zip(code_letter.keys(), code_letter.values())

Is roughly equivalent to

code_letter.items()

And just to make it a little clearer what the dictionary comprehension is doing, I might also rename loop variables:

letter_code = {letter: morse for morse, letter in code_letter.items()}

You're using bare excepts, which is generally a bad idea. If you accidentally typo some other catchable error into the try, you'll get 'Unknown value in string' messages instead of a real error. Specify what exact error you want to catch. I'd also make use of the fact that stringifying a KeyError exception tells you what the bad key was. You can print out the exception to tell the user what went wrong exactly

def morse_encode(string: str) -> str:
    try:
        return ' '.join(letter_code[i.upper()] for i in string)

    except KeyError as e:
        raise EncodeError(f'Unknown value in string: {e}')

>>> morse_encode("]")
EncodeError: Unknown value in string: ']'

And of course, i is a bad name for that loop variable. i suggests an index, but that's a letter, not an index. This reads much better

' '.join(letter_code[char.upper()] for char in string)
|improve this answer|||||
\$\endgroup\$
4
\$\begingroup\$

Some suggestions:

  1. I believe the idiomatic way to declare custom exceptions is to inherit from Exception rather than BaseException, and to just use the no-op pass as the class body. I've not seen the __module__ = Exception.__module__ pattern before, but if that does what it looks like (making the class effectively part of the exceptions module) that's probably going to be misleading. It's not a generic error, after all.
  2. It might be faster to put both upper and lower case characters in the dict than to use upper(). You could test this with timeit.
  3. If you're processing a big string of Morse code you might want to use an iterator. That way the string is processed one word at a time, rather than having to split the entire text into a list in memory before starting to output anything.
  4. Rather than parsing input() I would expect this sort of script to either read standard input or to accept a list of file path arguments which would each be read and converted.
  5. If the script also accepted --encode and --decode parameters (possibly with --encode as the default) it could easily be used to script a conversion of any text.
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ For the first point, if __module__ = Exception.__module__ is removed, the exception is shown as __main__.(something). Else it is shown as (somthing) \$\endgroup\$ – Srivaths Nov 20 '19 at 8:45

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.