HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Simple Morse code interpreter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codesimpleinterpretermorse

Problem

This is a simple Morse code interpreter with a CLI. It's a naive table-lookup implementation, and uses my-py for type checking. It automatically detects the input (English or Morse) and converts accordingly. It has the simplest possible CLI that I could think of. There is a git repo here.

I don't often write Python; I write Ruby for work and Clojure in my free time, but I do love Python and have a bit of experience with it. I welcome any suggestions on how to write more "Pythonic" code, simpler code or interfaces, or how to make the code more robust. Ideally, I want it to "just work" and be able to take even malformed data. Unfortunately, I think I need to keep the msg argument surrounded by quotes from the command line, otherwise the shell will interpret a single or double dot as the current or parent directory.

I'm considering using the Boyer-Moore algorithm instead of a lookup table, but I'm not sure if that's worth it.

`import re
import argparse
from typing import Dict

eng = {
"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" : "--..",
" " : "/",
"0" : "-----",
"1" : ".----",
"2" : "..---",
"3" : "...--",
"4" : "....-",
"5" : ".....",
"6" : "-....",
"7" : "--...",
"8" : "---..",
"9" : "----.",
"," : ",",
"." : ".",
"!" : "!",
"?" : "?",
} # type: Dict[str, str]

morse = {v: k for k, v in eng.items()} # type: Dict[str, str]

def morsep(msg: str) -> bool:
if msg[0][0] in ['.', '-', '/']:
return True
else:
return False

def morse_to_eng(msg: str) -> str:
out = "" # type: str
split_msg = re.

Solution

Overall, this code was very simple and easy to follow. I just have a few nit-picks to point out.

The function syntax

def func_name(param_name: type) -> return_type:


is new in Python 3. However, it is not a very common thing to be used in Python, and I don't think it looks very pythonic.

Your code will work just as well without making functions like this, so I recommend that you stick to using the usual syntax:

def func_name(param_name):


This clunky statement:

if msg[0][0] in ['.', '-', '/']:
    return True
else:
    return False


can be reduced to a more pythonic and clean version:

return msg[0][0] in ['.', '-', '/']


Comments like this:

# type: list


are really unnecessary and just add noise to your code. Just from reading your code, someone reviewing it should be able to determine what type each variable is.

You need to do some more error catching.

For example, say you are translating a string from morse code to english. What if you encounter a character that is not a valid morse character? Your program will raise an error here:

for char in msg:
    c = eng[char] # <======
    out += "{} ".format(c)


Therefore, this is what you should do:

-
Encase that line in a try/except.

-
In the except, throw a custom InvalidMorseLetter exception.

-
Exit

Here is what I mean:

try:
    c = eng[char] # type: str
except KeyError:
    raise(InvalidMorseLetter)


where InvalidMorseLetter is:

class InvalidMorseLetter(Exception):
    pass


Read here and here.

In the code, I wrote to catch a KeyError because that is the error that would be raised if an invalid dictionary key is used.

You should use a custom exception because it specifically describes what went wrong with your specific code, rather than just a general KeyError.

You should do something very similar to this for when you are translating english to morse.

Thanks to Zenohm for this.

In your translation functions, you should not be constantly modifying a single string each iteration:

c = morse[char] # type: str
out += c # <======================


The reason: python strings are immutable. Therefore, by constantly changing the string, you are actually creating a new string each and every time. This is very inefficient.

As Zenohm posted in the comments, you should be doing something along the lines of this:

return ''.join("{} ".format(eng[char]) for char in msg))



While the modifying of a single string does not a bad habit make and
isn't really a problem here, I think it would be good if he started
avoiding the modifying of an immutable object right off the bat.
return ''.join("{} ".format(eng[char]) for char in msg) Could be used
in place of his original string modification and would help to
encourage better practice in future.

Code Snippets

def func_name(param_name: type) -> return_type:
def func_name(param_name):
if msg[0][0] in ['.', '-', '/']:
    return True
else:
    return False
return msg[0][0] in ['.', '-', '/']
# type: list

Context

StackExchange Code Review Q#96717, answer score: 12

Revisions (0)

No revisions yet.