Morse code translator

Question

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 <iostream>
#include <map>
#include <string>
#include "split.h"

std::string SPACE (" ");
std::map<char, std::string> charMap;
std::map<std::string, char> 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<char, std::string>::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::string result;
    std::vector<std::string> morse = split(morseText, SPACE[0]);
    for (std::vector<std::string>::iterator i = morse.begin(); i != morse.end(); i++) {
        result.push_back(morseMap[*i]);
    }
    return result;
}

split.h

#ifndef SPLIT_H_INCLUDED
#define SPLIT_H_INCLUDED
#endif // SPLIT_H_INCLUDED

#include <string>
#include <sstream>
#include <vector>

std::vector<std::string> &split(const std::string &s, char delim, std::vector<std::string> &elems) {
    std::stringstream ss(s);
    std::string item;
    while (std::getline(ss, item, delim)) {
        elems.push_back(item);
    }
    return elems;
}


std::vector<std::string> split(const std::string &s, char delim) {
    std::vector<std::string> elems;
    split(s, delim, elems);
    return elems;
}

Concerns:

• Are the three generated lines necessary and are they used correctly?
• Is there a way to simplify the code?

Answer 1

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.

Answer 2

Regarding your concerns:

Is the three generated lines necessary?

They're called header (or #include) guards, and you can read about them here. They're used to avoid indirectly including copies of the same shared header code.

So, your code should look like this:

#ifndef SPLIT_H_INCLUDED
#define SPLIT_H_INCLUDED
// Put your translator.h code here
#endif // SPLIT_H_INCLUDED

Answer 3

As an ex Navy telegraphist, I can tell you that you are missing some characters commonly used in Morse. For example, to identify an error the code is 8 x e (eeeeeeee), to start and end the text of a message bt sent as a single character, to terminate a transmission ar sent as a single character plus others.

You may also want to take into account different transmission speeds. 12 words a minute broadcast sounds a whole lot different to a manual 30 words a minute from a professional.

You will have to teach it what a blank is, the space between words. An accurate example with translation can be seen in the Dam Busters film towards the end when the raid is happening.