11
\$\begingroup\$

ROT13 ("rotate by 13 places", sometimes hyphenated ROT-13) is a simple letter substitution cipher that replaces a letter with the letter 13 letters after it in the alphabet. ROT13 is a special case of the Caesar cipher, developed in ancient Rome.

ROT47 is a derivative of ROT13 which, in addition to scrambling the basic letters, also treats numbers and common symbols. Instead of using the sequence A–Z as the alphabet, ROT47 uses a larger set of characters from the common character encoding known as ASCII. Specifically, the 7-bit printable characters, excluding space, from decimal 33 '!' through 126 '~', 94 in total, taken in the order of the numerical values of their ASCII codes, are rotated by 47 positions, without special consideration of case. For example, the character A is mapped to p, while a is mapped to 2.

Below is my implementation of ROT47 algorithm:

#include <cstring>
#include <cstdio>
#include <iostream>

std::string rot47(std::string s)
{
    std::string s1 = "!\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
    std::string s2 = "PQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~!\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNO";

    std::string ret = "";
    for (unsigned int i = 0; i < s.size(); i++)
    {
        std::size_t pos = s1.find(s[i]);
        ret += s2[pos];
    }

    return ret;
}


int main()
{
    std::string str = "HelloWorld!";

    std::string retFct = rot47(str);
    std::cout << retFct << std::endl;

    std::string retFct2 = rot47(retFct);
    std::cout << retFct2 << std::endl;
}

It works as expected but my question is : Is there something I can improve in my implementation? I don't know if there is a more direct way to get the result.

\$\endgroup\$
  • \$\begingroup\$ Sorry if I'm missing something fundamental about ROT-47, but can't the new character just be determined with math rather than searching through a string? What you have is great for a general-purpose substitution cipher since you can change each of your strings to whatever you want, but ROT-47 can be much simpler, right? You just need to add 47 to the current character and then handle the wrap-around. \$\endgroup\$ – JPhi1618 Oct 6 '15 at 15:44
  • \$\begingroup\$ @JPhi1618 - that works if your system uses ASCII (which it probably does). It doesn't work with encodings such as EBCDIC. \$\endgroup\$ – Pete Becker Oct 6 '15 at 20:35
15
\$\begingroup\$
#include <cstring>
#include <cstdio>

You're not using either of those, so remove them, and include <string> instead to get std::string.

std::string rot47(std::string s) { /* ... */ }

Why copy the caller's string? You're only reading from it, so take it by const&.

std::string s1 = "!\"#$%&\...";
std::string s2 = "PQRSTUVW...";

These two are constant, don't allocate them for every call, make them static (and const):

static std::string const s1 = "...";
std::string ret = "";

That initialization is not necessary, a default-constructed string is empty. You might want to reserve the appropriate size since you know it though, for optimal performance.

for (unsigned int i = 0; i < s.size(); i++) { /* ... */ }

You can use range-based for loop with strings:

for (auto c: s) { /* ... */ }

And you should probably check that s1.find actually found something, otherwise the line after that is undefined behavior.

And s, s1 and s2 are rather poor choices for those variable names. Something like "source", "original", "plaintext" for the input would be better. map_from / map_to could work for the other two.

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for the last paragraph on variable names. FAR too few people actually worry enough about these. \$\endgroup\$ – Nzall Oct 6 '15 at 11:32
5
\$\begingroup\$

EBCDIC is a myth made up to scare children during holidays, so here is the ASCII-dependent version, as suggested in a comment by JPhi1618.

The main imrovement is to efficiency, by avoiding std::string::find, which is probably implemented as a linear search.

Unicode is not a myth, unfortunately, so don't ever try encrypting the string "Zurück".

    #include <string>
    #include <iostream>

    std::string rot47(const std::string& s) {
        std::string ret;
        for(const char plain : s) {
            const char crypt = '!' + (plain - '!' + 47) % 94;
            ret += crypt;
        }
        return ret;
    }

    int main() {
        std::string str = "HelloWorld!";
        std::string retFct = rot47(str);
        std::cout << retFct << std::endl;
        std::string retFct2 = rot47(retFct);
        std::cout << retFct2 << std::endl;
    }
\$\endgroup\$
  • \$\begingroup\$ That is a nice way to do it, thank you for your answer. ;) \$\endgroup\$ – Aleph0 Oct 7 '15 at 8:14
  • \$\begingroup\$ Generally a nice answer, but it fails to correctly 'round trip' space characters - they get transformed back into '~' characters. As written, might want to add if (plain == ' ') ret+= plain; else... \$\endgroup\$ – DaveParillo Nov 11 '16 at 18:07
  • \$\begingroup\$ or anchor the rotation on character 32: ' ' instead of 33: '!' \$\endgroup\$ – DaveParillo Nov 11 '16 at 18:18
  • \$\begingroup\$ @DaveParillo This was just intended to duplicate how the OP's code behaves, but without using a linear search through the alphabet. If you pass a space to that code (as it is now), a character gets accessed at index npos in one of the strings, so yeah. \$\endgroup\$ – Christopher Oicles Nov 11 '16 at 22:15
  • \$\begingroup\$ Actually, you implementation does not exactly duplicate the OP's behavior. Their substitution strings don't include a space at all, so one of the side-effects of their implementation is that all spaces are removed after two rotations. Your implementation replaces them with tilde's \$\endgroup\$ – DaveParillo Nov 12 '16 at 1:33
4
\$\begingroup\$

It's always a good idea to try to avoid explicit for loops. Indeed, you can do this achieve this nicely using std::transform as follows.

#include <string>
#include <iostream>
#include <algorithm>

void inplace_rot47(std::string& s)
{
   std::transform(s.begin(), s.end(), s.begin(), 
     [&](char plain) { return '!' + (plain - '!' + 47) % 94; });
}

int main() 
{
   std::string s = "HelloWorld!";

   inplace_rot47(s);
   std::cout << s << std::endl;

   inplace_rot47(s);
   std::cout << s << std::endl;
}

Of course, if you wanted to, you could return a new string rather than write over the string. For this, you'd want use an iterator pushing to a new string (the third argument of std::transform).

\$\endgroup\$
  • 1
    \$\begingroup\$ That's a nice way to avoid my explicit loop. Thank you for your answer. ;) \$\endgroup\$ – Aleph0 Oct 19 '15 at 8:56
2
\$\begingroup\$

As I see this, your code would behave strangely when fed non-ASCII characters. It iterates through all bytes in the string, and will thus do strange things with multibyte-characters like in the UTF-8 encoding.

You could try it yourself by converting "Zurück" with the code.

This isn't that easy of a fix unfortunatly. You'd either need to explicitly support UTF-8, or do hacky things like aborting/erroring when a character greater than 0x7F is inputted (because you can't know the length of the following bytes).

\$\endgroup\$
  • \$\begingroup\$ This is a good catch, but given that ROT47 is based on ASCII, I think the best fix is documenting that this function is intended for use with ASCII strings and behavior with other encodings is undefined. Trying to detect different encodings based on byte patterns is starting down a very, very deep rabbit hole. \$\endgroup\$ – GrandOpener Oct 6 '15 at 15:16
  • \$\begingroup\$ @GrandOpener - but if you require ASCII for input the implementation can be much simpler: just add 47, and adjust if the result is too large. \$\endgroup\$ – Pete Becker Oct 6 '15 at 20:37
  • \$\begingroup\$ @PeteBecker - Yes, I completely agree. If I were writing an answer, I would include that suggestion. \$\endgroup\$ – GrandOpener Oct 6 '15 at 21:41
  • \$\begingroup\$ I know it won't work for non-ASCII characters but this is related to the algorithm itself. Anyway, I didn't plan to use it on non-ASCII characters in my case, thus it's not really a problem for me. Thank you for your answer. :) \$\endgroup\$ – Aleph0 Oct 7 '15 at 8:18

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.