patterncppModerate
Morse code translator
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
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
Use objects
The
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
The
Pass complex objects by reference
The
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
Eliminate
The one place that
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 practicalThe
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
splitThe 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.