1
\$\begingroup\$

I have written a few C++ functions that should:

  • take in a string
  • transform it based on index
  • transform it based on number of alphabetic characters seen
  • return the transformed string

Example first, detail after:

$ rotn_a("Hello, World!")
> Igopt, Exbwp!

$ rotn_a("aaaaaaaaaaaaaaaaaaaaaaaaaaa")
> bcdefghijklmnopqrstuvwxyzab

$rotn_a("aa!aaaaaaaaaaaaaaaaaaaaaaaa")
> bc!efghijklmnopqrstuvwxyzab
     ^


$rotn_b("Hello, World!")
> Igopt, Cvzun!

$rotn_b("aaaaaaaaaaaaaaaaaaaaaaaaaaa")
> bcdefghijklmnopqrstuvwxyzab

$rotn_b("aa!aaaaaaaaaaaaaaaaaaaaaaaa")
> bc!defghijklmnopqrstuvwxyza
     ^


$unrotn_a(rotn_a("Hello, World!"))
> Hello, World!

$unrotn_a(rotn_a("aaaaaaaaaaaaaaaaaaaaaaaaaaa"))
> aaaaaaaaaaaaaaaaaaaaaaaaaaa

$unrotn_a(rotn_a("aa!aaaaaaaaaaaaaaaaaaaaaaaa"))
> aa!aaaaaaaaaaaaaaaaaaaaaaaa


$unrotn_b(rotn_b("Hello, World!"))
> Hello, World!

$unrotn_b(rotn_b("aaaaaaaaaaaaaaaaaaaaaaaaaaa"))
> aaaaaaaaaaaaaaaaaaaaaaaaaaa

$unrotn_b(rotn_b("aa!aaaaaaaaaaaaaaaaaaaaaaaa"))
> aa!aaaaaaaaaaaaaaaaaaaaaaaa

rotn_a: take in string and rotate each each character index times rotn_a example:

in:  aaaaa

middle step
-----------

     bbbbb
     bcccc
     bcddd
     bcdee
     bcdef

out: bcdef

unrotn_a: undo rotn_a

unrotn_a example:

in:  bcdef

middle step
-----------

     abcde
     aabcd
     aaabc
     aaaab

out: aaaaa

rotn_b: take in string and rotate each each character index-alpha_encountered times and alpha_encountered is number of a-z seen in string so far rotn_b example:

in:  aa'aa

middle step
-----------

     bb'bb
     bc'cc
     bc'cc
     bc'dd

out: bc'de

unrotn_b: undo rotn_b

#include <iostream>
#include <algorithm>

std::string rotn_a(std::string text);
std::string rotn_b(std::string text);

std::string unrotn_a(std::string text);
std::string unrotn_b(std::string text);

int main() {
    // Encode example
    std::cout
        << rotn_a("Hello, World!") << "\n"
        << rotn_a("aaaaaaaaaaaaaaaaaaaaaaaaaaa") << "\n"
        << rotn_a("aa!aaaaaaaaaaaaaaaaaaaaaaaa") << "\n\n";
    std::cout
        << rotn_b("Hello, World!") << "\n"
        << rotn_b("aaaaaaaaaaaaaaaaaaaaaaaaaaa") << "\n"
        << rotn_b("aa!aaaaaaaaaaaaaaaaaaaaaaaa") << "\n\n";

    // Decode example
    std::cout
        << unrotn_a(rotn_a("Hello, World!")) << "\n"
        << unrotn_a(rotn_a("aaaaaaaaaaaaaaaaaaaaaaaaaaa")) << "\n"
        << unrotn_a(rotn_a("aa!aaaaaaaaaaaaaaaaaaaaaaaa")) << "\n\n";
    std::cout
        << unrotn_b(rotn_b("Hello, World!")) << "\n"
        << unrotn_b(rotn_b("aaaaaaaaaaaaaaaaaaaaaaaaaaa")) << "\n"
        << unrotn_b(rotn_b("aa!aaaaaaaaaaaaaaaaaaaaaaaa")) << "\n\n";
}

std::string rotn_a(std::string text) {
    for (auto i = begin(text); i != end(text); ++i) {
        // Modified from https://codereview.stackexchange.com/a/14610/177972
        std::transform(
            i, end(text), i,
            [] (char c) -> char {
                if (not std::isalpha(c)) {
                    return c;
                }

                char const pivot = std::isupper(c) ? 'A' : 'a';
                return (c - pivot + 1) % 26 + pivot;
            }
        );
    }

    return text;
}

std::string rotn_b(std::string text) {
    auto rot = 0;

    for (auto i = begin(text); i != end(text); ++i) {

        auto c = *i;

        if (not std::isalpha(c)) {
            continue;
        }

        rot++;
        char const pivot = std::isupper(c) ? 'A' : 'a';
        *i = (c - pivot + rot) % 26 + pivot;
    }

    return text;
}

std::string unrotn_a(std::string text) {
    for (auto i = begin(text); i != end(text); ++i) {
        std::transform(
            i, end(text), i,
            [] (char c) -> char {
                if (not std::isalpha(c)) {
                    return c;
                }

                char const pivot = std::isupper(c) ? 'A' : 'a';
                return (c - pivot + 25) % 26 + pivot;
            }
        );
    }

    return text;
}

std::string unrotn_b(std::string text) {
    auto rot = 0;

    for (auto i = begin(text); i != end(text); ++i) {

        auto c = *i;

        if (not std::isalpha(c)) {
            continue;
        }

        rot++;
        char const pivot = std::isupper(c) ? 'A' : 'a';
        *i = (c - pivot + (26-rot)) % 26 + pivot;
    }

    return text;
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Would the person that voted to close this question please explain why they think the code is incomplete or not working. \$\endgroup\$
    – pacmaninbw
    Commented Apr 1, 2020 at 1:14

1 Answer 1

1
\$\begingroup\$

I'm just going to review rotn_a, but these comments apply to all of the functions.


std::string rotn_a(std::string text);
...
std::string rotn_a(std::string text) { ...

There's nothing wrong with separating the definition and declaration, but I don't think it gets you anything in this example.


std::string rotn_a(std::string text) {

In this case, you don't actually need a copy. You just need a new string of the same size. It may be worthwhile to make a const& version and a && version.


for (auto i = begin(text); i != end(text); ++i) {
    // Modified from https://codereview.stackexchange.com/a/14610/177972
    std::transform(
        i, end(text), i,

Imagine you have a string "qwertyuiop" ... here's what your program will do:

q w e r t y u i o p
>>>>>>>>>>>>>>>>>>v
  v---------------+
  >>>>>>>>>>>>>>>>v
    v-------------+
    >>>>>>>>>>>>>>v
      v-----------+
      >>>>>>>>>>>>v
        v---------+
        >>>>>>>>>>v
...

You go over the entire string many many times. O(n^2) times. It's very easy to make this function go over the string once.


std::transform(i, end(text), i, ...

Minor nit: It's good to get in the habit of using std:: everywhere, but if you choose not to, then you should be consistent.


if (not std::isalpha(c))

Minor nit 2: I think if you explained this function to a human, you would say "do blah blah blah to the letters." You probably wouldn't say "first ignore the non-letters. Then with the remaining letters, do blah blah blah."

So an (IMO) improved version would be:

if (std::isalpha(c)) {
    ...
}

 char const pivot = std::isupper(c) ? 'A' : 'a';

I think what you wrote is good. If you want to go crazy, you could maybe optimize it to something like char pivot = 01000001 | (C & 00100000). I don't know for sure that the bitwise version is faster, but I suspect it would be since isupper is a function call that (IIRC) needs to check the current locale which basically means it won't be inlined.

Also I think pivot is not a great name. Maybe "base letter" or something like that. Pivot makes me thing you are going to do comparisons and expect to have some things less than pivot and some things greater than pivot.

Since many of your functions do something like this, perhaps you could factor it out and make it a helper function.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.