Return to Revisions

File organization

You should have three files:

• bitspeak.hpp, the header containing only the minimally necessary definitions
• bitspeak.cpp, the implementation of the functions defined in the header
• bitspeak_test.cpp, the tests for the implementation

bitspeak.hpp

#ifndef BITSPEAK_HPP
#define BITSPEAK_HPP

#include <string>

namespace bitspeak
{
    std::string encode(const std::string &data);
    std::string decode(const std::string &text);
}

#endif

This is the simplest definition of the interface that a caller needs. The std::string class can handle all bytes, therefore it is suitable both for strings of char, as well as for byte arrays.

bitspeak.cpp

Instead of writing the vows and cons in a single line, I rather like them to look like this:

static const std::string vows[32] = {
    "a",  "e",  "i",  "o",  "u",  "w",  "y",  "ae",
    "ai", "ao", "au", "aw", "ay", "ea", "ee", "ei",
    "eo", "eu", "ey", "ia", "io", "iu", "oa", "oi",
    "ou", "ow", "oy", "ui", "uo", "uy", "we", "wi"
};
static const std::string cons[128] = {
    "b",   "c",   "d",   "f",   "g",   "h",   "j",   "k",
    "l",   "m",   "n",   "p",   "q",   "r",   "s",   "t",
    "v",   "x",   "z",   "bc",  "bl",  "br",  "bs",  "ch",
    "chs", "cht", "ck",  "cl",  "cr",  "cs",  "ct",  "fl",
    "fr",  "fs",  "ft",  "gl",  "gm",  "gr",  "gs",  "gz",
    "hd",  "hf",  "hr",  "hs",  "ht",  "hv",  "jr",  "js",
    "jt",  "kl",  "kr",  "kt",  "lb",  "lc",  "ld",  "lg",
    "lm",  "ln",  "lp",  "ls",  "lt",  "lv",  "mb",  "mn",
    "mp",  "mr",  "nd",  "nk",  "nm",  "np",  "ns",  "nt",
    "nv",  "nz",  "pl",  "pr",  "ps",  "pt",  "pz",  "rb",
    "rc",  "rch", "rd",  "rf",  "rg",  "rk",  "rl",  "rm",
    "rn",  "rp",  "rr",  "rs",  "rt",  "rv",  "rz",  "sb",
    "sc",  "sch", "sd",  "sf",  "sh",  "sl",  "sm",  "sn",
    "sp",  "sq",  "sr",  "st",  "sth", "str", "sv",  "sz",
    "tch", "th",  "tl",  "tn",  "tr",  "ts",  "vl",  "vr",
    "vs",  "vz",  "xc",  "xz",  "zd",  "zl",  "zp",  "zt"
};

Grouping them in 8 per line lets the reader quickly check that these are indeed 32 respectively 128 strings.

The implementation of bitspeak::encode can be done in a single pass without unnecessary copying:

namespace bitspeak {

std::string encode(const std::string &data)
{
    std::ostringstream out;

    std::size_t len = data.length();
    for (std::size_t i = 0; i < len; i += 3) {
        uint8_t d0 = data[i + 0];
        uint8_t d1 = i + 1 < len ? data[i + 1] : 0;
        uint8_t d2 = i + 2 < len ? data[i + 2] : 0;

        uint8_t cc0 = (d0 & 0xFE) >> 1;
        uint8_t vv0 = ((d0 & 0x01) << 4) | ((d1 & 0xF0) >> 4);
        uint8_t cc1 = ((d1 & 0x0F) << 3) | ((d2 & 0xE0) >> 5);
        uint8_t vv1 = d2 & 0x1F;

        out << cons[cc0];
        out << vows[vv0];
        out << (i + 1 < len ? cons[cc1] : "?");
        out << (i + 2 < len ? vows[vv1] : "?");
    }

    return out.str();
}

} // end of namespace

I have changed the following things:

• There is a single loop, which processes 3 bytes at a time (note that i += 3 is used here instead of the usual i++).
• Instead of using uint32_t, the code uses std::size_t, which is exactly the type returned by std::string.length().
• The first block that defines d0, d1 and d2 makes sure that the data bytes are interpreted as unsigned.
• They are then converted in the same way that you used in the ca3_to_ca4 function.
• The last block appends the consonants and vowels. Even in the last group, the first pair of consonants and vowels is always relevant, since the bits from d0 influence cc0 and vv0. The remaining characters may need to be replaced with the padding character.
• The class std::ostringstream is suitable for efficient appending to a string.

For decoding a string of consonants and vowels into the original data, idiomatic C++ code does not copy much data around. Instead, it uses indexes or iterators into the strings, which are only pointers that can be efficiently passed around.

The basic idea is this:

• find a sequence of consonants
• find a sequence of vowels
• find a sequence of consonants
• find a sequence of vowels
• map the above sequences to indices
• bitshift the indices around to produce the original data
• repeat until the end of the encoded string

The padding needs to be handled specially since it is only allowed at the very end. And this is the code I came up with:

std::string decode(const std::string &text)
{
    std::size_t i = 0;
    std::size_t len = text.length();

    std::ostringstream out;

    while (i < len) {
        auto cc0start = i;
        while (i < len && con_l.find(text[i]) != std::string::npos)
            i++;
        auto vv0start = i;
        while (i < len && vow_l.find(text[i]) != std::string::npos)
            i++;
        auto cc1start = i;
        while (i < len && con_l.find(text[i]) != std::string::npos)
            i++;
        auto vv1start = i;
        while (i < len && vow_l.find(text[i]) != std::string::npos)
            i++;
        if (i == cc0start)
            throw "invalid encoded data (neither consonant nor vowel)";

        auto cc0 = text.substr(cc0start, vv0start - cc0start);
        auto vv0 = text.substr(vv0start, cc1start - vv0start);
        auto cc1 = text.substr(cc1start, vv1start - cc1start);
        auto vv1 = text.substr(vv1start, i - vv1start);

        auto cons_end = cons + 128;
        auto vows_end = vows + 32;
        auto cc0bits = std::find(cons, cons_end, cc0) - cons;
        auto vv0bits = std::find(vows, vows_end, vv0) - vows;
        auto cc1bits = std::find(cons, cons_end, cc1) - cons;
        auto vv1bits = std::find(vows, vows_end, vv1) - vows;

        if (cc0bits == 128 || vv0bits == 32)
            throw "invalid encoded data (unknown letter combination)";
        unsigned char d0 = (cc0bits << 1) | ((vv0bits & 0x10) >> 4);
        out << d0;

        if (cc1bits == 128) {
            if (text.substr(i) != "??")
                throw "invalid encoded data (padding)";
            break;
        }
        unsigned char d1 = ((vv0bits & 0x0F) << 4) | ((cc1bits & 0x78) >> 3);
        out << d1;

        if (vv1bits == 32) {
            if (text.substr(i) != "?")
                throw "invalid encoded data (padding)";
            break;
        }
        unsigned char d2 = ((cc1bits & 0x07) << 5) | (vv1bits & 0x1F);
        out << d2;
    }

    return out.str();
}

I know that the above code violates some best practice rules:

• it contains duplicate code in the while loops. I did this since extracting the loops to a separate function would add even more code.
• it contains hard-coded numbers, which some readers may consider magic numbers. But since they are powers of 2 and mentioned in the specification, they are not magic, they are only numbers, like the other numbers in the bit-shifting code.
• these numbers are even repeated in several places. I did not make them constants since they will not change anyway. And even if they should do, each 128 in the code has exactly the same meaning, so they can all be changed doing a simple text replacement.

Your is_vowel function can be shortened a bit.

bool is_vowel(char x) { bool o = false; std::size_t i; for (i = 0; i < vow_l.size(); i++) { if (x == vow_l[i]) { o = true; break; } } return o; }

bool is_vowel(char x) { std::size_t i; for (i = 0; i < vow_l.size(); i++) { if (x == vow_l[i]) { return true; } } return false; }

bool is_vowel(char x) { for (std::size_t i = 0; i < vow_l.size(); i++) { if (x == vow_l[i]) { return true; } } return false; }

bool is_vowel(char x) { return vow_l.find(x) != std::string::npos; }

Instead of using break and a return value, it is often simpler to just return value, e.g. in string_find.