17
\$\begingroup\$

As the first step for writing a Morse code practice program, I thought I'd start with the very simplest step of a text-based Morse code translator. This simple program reads lines from stdin and emits them as a text-based representation of the Morse code equivalent. Any character that doesn't have a Morse code equivalent is simply skipped.

I'm interested particularly in a few things.

  1. Is there a better representation than map that I should use?
  2. Is there a way to declare it as constexpr?
  3. Would this be better implemented as a facet?

The private emit member function is intended to be replaced by a mechanism that will queue up audio output.

Morse.cpp:

#include <iostream>
#include <sstream>
#include <cctype>
#include <map>

/* Morse code ITU-4 M.1677 */

class Morse
{
public:
    Morse &operator<<(char ch) { emit(ch); return *this; }
    Morse &operator<<(const std::string &msg)  
    {
        for (const auto &ch : msg) 
            emit(ch);
        return *this;
    }
private:
    void emit(const char &ch) const {
        std::cout << coding[std::toupper(ch)] << ' ';
    }

static std::map<const char, const std::string> coding;
};

std::map<const char, const std::string> Morse::coding = {
    {' ', " "},
    {'\n', "\n"},
    {'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', "-----"},
    {'.', ".-.-.-"},
    {',', "--..--"},
    {'?', "..--.."},
    {'-', "-...-"},
    {'/', "-..-."},
    {'@', ".--.-."},
    {'+', ".-.-."},
    {'=', "-..."},
    {'\'', ".----"},
    {'(', "-.--"},
    {')', "-.--."},
    {'\"', ".-..-"},
    {'\x04', "...-.-"}, //EOT = SK
};

int main()
{
    using namespace std;

    Morse morse;
    string line;
    while (getline(cin, line))
        morse << line << '\n';
}
\$\endgroup\$
  • 3
    \$\begingroup\$ And concerning facets, all I can say is that few people will be able to help you. That's one of the least used and most obscure corners of the standard :p \$\endgroup\$ – Morwenn Nov 30 '14 at 14:47
  • 1
    \$\begingroup\$ facets are fun: Better? They will provide a way to gurantee that all text on the stream is Morse enoded. If you want this gurantee then sure using facets is an option. Once a stream is embued with a facet it can not be undone (in most cases). \$\endgroup\$ – Martin York Nov 30 '14 at 18:43
  • \$\begingroup\$ I don't know if that answers your question but wouldn't a hash map be more efficient than a map? \$\endgroup\$ – Kam Dec 7 '14 at 5:42
12
\$\begingroup\$

The others have provided enough comments on the current code.

I find using facets relatively straight forward.

int main()
{
    std::locale         MorseLocal(std::locale::classic(), new ASCIItoMorse());

    /* Imbue std::cout before it is used */
    std::cout.imbue(MorseLocal);

    std::cout << "Line 1\nLine 2\nLine 3\n";

    /* You must imbue a file stream before it is opened. */
    std::ofstream       data;
    data.imbue(MorseLocal);
    data.open("PLOP");

    data << "Loki\n translated into morse rather than norse.";
}

Then the facet you want to specialize is codecvt

#include <locale>
#include <algorithm>
#include <iostream>
#include <fstream>

class ASCIItoMorse: public std::codecvt<char,char,std::mbstate_t>
{
  public:
   explicit ASCIItoMorse(size_t ref = 0): std::codecvt<char,char,std::mbstate_t>(ref)    {}

    typedef std::codecvt_base::result               result;
    typedef std::codecvt<char,char,std::mbstate_t>  parent;
    typedef parent::intern_type                     intern_type;
    typedef parent::extern_type                     extern_type;
    typedef parent::state_type                      state_type;
  protected:
    virtual result do_out(state_type& tabNeeded,
                         const intern_type* rStart, const intern_type*  rEnd, const intern_type*&   rNewStart,
                         extern_type*       wStart, extern_type*        wEnd, extern_type*&         wNewStart) const
    {
        result  res = std::codecvt_base::ok;

        for(;rStart < rEnd;++rStart)
        {
            // Get the output string
            // Note: I would add the trailing space into each `coding` value.
            std::string const&  output = coding[std::toupper(*rStart)];
            // Check we have room
            if (wStart+output.size() > wEnd)
            {
                res = std::codecvt_base::partial;
                break;
            }
            wStart = std::copy(std::begin(output), std::end(output), wStart);
        }

        rNewStart   = rStart;
        wNewStart   = wStart;

        return res;
    }

    // Override so the do_out() virtual function is called.
    virtual bool do_always_noconv() const throw()  {return false;}
};

Output:

> ./a.out
.-.. .. -. .  .----
.-.. .. -. .  ..---
.-.. .. -. .  ...--
> cat PLOP
.-.. --- -.- ..
 - .-. .- -. ... .-.. .- - . -..  .. -. - ---  -- --- .-. ... .  .-. .- - .... . .-.  - .... .- -.  -. --- .-. ... . .-.-.- 

Some examples of fun things to do with Facets and locale's:

Facet: numpunct

Facet: ctype

Facet: codecvt

\$\endgroup\$
  • 3
    \$\begingroup\$ That's my first time reading something that straightforward about facets. Thanks! \$\endgroup\$ – Morwenn Nov 30 '14 at 22:15
6
\$\begingroup\$

Concerning the map, I don't think that there is any way to make it constexpr since it allocates memory dynamically and there is no such thing as predynamic storage (constexpr new and delete) in the C++ standard (yet?).

You could create a constexpr mapping with a simple array (or std::array), using characters as indices and const char* as values. But you would probably have to fill every ASCII values with some values, even those you don't use. That's not the cleanest thing to do.

Concerning the choice of the mapping, if I remember correctly, std::unordered_map has not the best insertion time in the world, but has a really efficient \$O(1)\$ lookup. Since you initialize it once and then you only perform lookups, such a construct could be more efficient in your case. On the other hand, you could try to use a boost::flat_map which is a map based on a vector and which is consequently cache-friendly. For almost anything, the good old std::map is the slowest alternative.

\$\endgroup\$
  • 1
    \$\begingroup\$ slowest: true. But in relative terms when you have only 51 elements how slow is that compared to the rest of the code and is it really the bottleneck you want to optimize? I don't know but a question worth asking. \$\endgroup\$ – Martin York Nov 30 '14 at 18:46
  • 1
    \$\begingroup\$ When the program is emitting audio, it will be even slower, so performance isn't an issue in this instance, but it's worth considering generally. \$\endgroup\$ – Edward Nov 30 '14 at 18:48
  • \$\begingroup\$ @LokiAstari That's true, but the questions about constexpr and "a better representation" made me feel that the question was about efficiency. \$\endgroup\$ – Morwenn Nov 30 '14 at 22:09
  • \$\begingroup\$ If you "fill every ASCII values", then the program won't run correctly on non-ASCII systems, so that's another drawback to the const array idea. The point about preferring unordered map for is a good one. \$\endgroup\$ – Toby Speight Oct 26 '17 at 11:07
6
\$\begingroup\$

#include <sstream> is superfluous.

Morse is hard-coded to only emit its output to std::cout. It would be nice if cout were an overridable default.

Your treatment of '\n' is incorrect, in my opinion. A newline is rendered as a newline followed by a space. I believe that the space should not be emitted.

As always, I advise against omitting optional braces, as the maintenance pitfall is not worth the space savings.

\$\endgroup\$
6
\$\begingroup\$

Purely aesthetic changes:

  • I would align the second } in the initializer list of the coding map:

    std::map<const char, const std::string> Morse::coding = {
        { ' ' , " "    },
        { '\n', "\n"   },
        { 'A' , ".-"   },
        { 'B' , "-..." },
        { 'C' , "-.-." },
        { 'D' , "-.."  },
        // you get the idea...
    

    Also, the last value in the list doesn't have to be followed by a , (this is a warning if your compiler's warning levels are high enough). It is not, @Kevin is right. Not even Clang's -Weverything produced a warning for it. The trailing comma is allowed by the language, so no warning is required.

  • Curly braces positioning in your class methods is not uniform. You actually have three styles: Everything in the same line, { in its own line and { in the same line as the function header. It would be best to make it uniform throughout.

Other details:

  • You can make the coding map a const. Not as good as a constexpr perhaps, but will prevent accidental modification of the map and make your intentions clear to any reader.

  • void emit(const char &ch) has no reason to take the char parameter by const ref. Take it by value, since it is a register-sized native type.

  • By accessing the map via the array subscript operator in emit(): coding[std::toupper(ch)] you will be creating a new map entry if a char that is not mapped is requested. To prevent that, find() the requested char and return a default/invalid or throw and exception if requested entry doesn't exist. std::map::at() might also be a better option.

\$\endgroup\$
  • 5
    \$\begingroup\$ "Also, the last value in the list should not be followed by a , (this is a warning if your compiler's warning levels are high enough)." - What compiler is that? Neither G++ nor clang do, with warnings all the way up. And it's arguably better to keep it, it simplifies the change list when you want to add items at the end. \$\endgroup\$ – Kevin Nov 30 '14 at 18:00
  • \$\begingroup\$ @Kevin, indeed you are correct, I was under the impression that one of the "paranoid" flags of GCC or Clang would produce a warning for that. \$\endgroup\$ – glampert Nov 30 '14 at 18:32
  • 1
    \$\begingroup\$ @glampert It's allowed by the language. The only reason the compiler would produce a diagnostic is if the programmer was doing something unintentional, but the stray comma isn't really harmful. \$\endgroup\$ – user25057 Dec 1 '14 at 0:25
  • \$\begingroup\$ @remyabel, yes, correct. I've looked into that. The trailing comma is legal and requires no diagnostics message. \$\endgroup\$ – glampert Dec 1 '14 at 0:47

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.