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

Morse code translator

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

Problem

I have a Morse Code translator, which can translate text to Morse code and vice-versa. Note that for the two files that the first three lines are generated by the Code::Blocks IDE.

translator.h

```
#ifndef SPLIT_H_INCLUDED
#define SPLIT_H_INCLUDED
#endif // SPLIT_H_INCLUDED

#include
#include
#include
#include "split.h"

std::string SPACE (" ");
std::map charMap;
std::map morseMap;
bool hasInit = false;

void init()
{
charMap['a'] = ".-";
charMap['b'] = "-...";
charMap['c'] = "-.-.";
charMap['d'] = "-..";
charMap['e'] = ".";
charMap['f'] = "..-.";
charMap['g'] = "--.";
charMap['h'] = "....";
charMap['i'] = "..";
charMap['j'] = ".---";
charMap['k'] = "-.-";
charMap['l'] = ".-..";
charMap['m'] = "--";
charMap['n'] = "-.";
charMap['o'] = "---";
charMap['p'] = ".--.";
charMap['q'] = "--.-";
charMap['r'] = ".-.";
charMap['s'] = "...";
charMap['t'] = "-";
charMap['u'] = "..-";
charMap['v'] = "...-";
charMap['w'] = ".--";
charMap['x'] = "-..-";
charMap['y'] = "-.--";
charMap['z'] = "--..";
charMap['0'] = "-----";
charMap['1'] = ".----";
charMap['2'] = "..---";
charMap['3'] = "...--";
charMap['4'] = "....-";
charMap['5'] = ".....";
charMap['6'] = "-....";
charMap['7'] = "--...";
charMap['8'] = "---..";
charMap['9'] = "----.";
charMap['.'] = ".-.-.-";
charMap[','] = "--..--";
charMap['?'] = "..--..";
charMap[' '] = "/";
for (std::map::iterator i = charMap.begin(); i != charMap.end(); i++) {
morseMap[i -> second] = i -> first;
}
}

std::string translateToMorse(std::string plainText)
{
if (!hasInit) {
init();
}
std::string result;
for (int i = 0; plainText[i]; i++) {
result.append(charMap[plainText[i]]);
result.append(SPACE);
}
return result;
}

std::string translateFromMorse(std::string morseText)
{
if (!hasInit) {
init();
}
std::s

Solution

I see a number of things that may help you improve your code.

Headers should not contain code

Header files in C++ should generally only contain other headers, definitions and structure and class definitions. They should not contain code implementations, so neither of these .h files should really be .h files. Instead they should both be .cpp files because they contain code implementations or they should be split into separate declaration/implementation (.h/.cpp) pieces.

Use objects

The init() function seems to me that it would be better implemented as an object constructor and then the individual functions could be static member functions. Here's another way to do Morse conversion.

Eliminate global variables

There is really no reason here to have global variables. Instead, these should be wrapped into an object, or at least a namespace.

Use const where practical

The SPACE variable should be const as should be the arguments to the functions.

Pass complex objects by reference

The std::string arguments to either of the functions should be a reference to avoid an unnecessary copy. That is the function could instead be:

std::string translateToMorse(const std::string &plainText)


Consider the user

If someone very reasonably tries to translate a callsign such as W1AW they'll get back the string ".----" which is only the Morse code for '1'. The problem is that the code only accepts lowercase characters and neither throws an error nor gives any indication that the uppercase characters were simply ignored. That's not very helpful to the user. Consider instead throwing an exception. Simply ignoring characters is unlikely to be a desirable behavior in any case.

Eliminate split

The one place that split is used is in the translateFromMorse routine, but it's really not needed there. Instead you could introduce new functions that would translate a word at a time and use std::stringstream to extract a word at a time for translation.

Code Snippets

std::string translateToMorse(const std::string &plainText)

Context

StackExchange Code Review Q#110768, answer score: 13

Revisions (0)

No revisions yet.