caesar cipher (rot<2-25>)

Question

I'm a rookie in c++ programming. Recently, caesar ciphers caught my eye. I ended up making a rot<N> text converter, where N is any value between 2 and 25. Please keep in mind that I'm still new to programming and don't know advanced level stuff. All your feedbacks will be appreciated. FYI, this was developed in Visual Studio 2019.

#include <iostream>
#include <string>

std::string user_input;//user input string
std::string output_str;//output ciphertext
int x;//loop counter 
int corres_num[1500000] = {};//corresponding number according to alphabets
int cipher_value;//the cipher rotation value

void termclr()//terminal clear function
{
#ifdef __linux__
system("clear");
#elif _WIN32
system("cls");
#endif
}

void convertion()//user input convertion
{
std::string output_str_assign = "";//assigns individual ciphered characters to main ciphertext
int calc = 0;//contributes in assigning <output> 
int output = 0;//assigns individual cipher characters
if (user_input[x] == 'A' || user_input[x] == 'a') { corres_num[x] = 1; }
else if (user_input[x] == 'B' || user_input[x] == 'b') { corres_num[x] = 2; }
else if (user_input[x] == 'C' || user_input[x] == 'c') { corres_num[x] = 3; }
else if (user_input[x] == 'D' || user_input[x] == 'd') { corres_num[x] = 4; }
else if (user_input[x] == 'E' || user_input[x] == 'e') { corres_num[x] = 5; }
else if (user_input[x] == 'F' || user_input[x] == 'f') { corres_num[x] = 6; }
else if (user_input[x] == 'G' || user_input[x] == 'g') { corres_num[x] = 7; }
else if (user_input[x] == 'H' || user_input[x] == 'h') { corres_num[x] = 8; }
else if (user_input[x] == 'I' || user_input[x] == 'i') { corres_num[x] = 9; }
else if (user_input[x] == 'J' || user_input[x] == 'j') { corres_num[x] = 10; }
else if (user_input[x] == 'K' || user_input[x] == 'k') { corres_num[x] = 11; }
else if (user_input[x] == 'L' || user_input[x] == 'l') { corres_num[x] = 12; }
else if (user_input[x] == 'M' || user_input[x] == 'm') { corres_num[x] = 13; }
else if (user_input[x] == 'N' || user_input[x] == 'n') { corres_num[x] = 14; }
else if (user_input[x] == 'O' || user_input[x] == 'o') { corres_num[x] = 15; }
else if (user_input[x] == 'P' || user_input[x] == 'p') { corres_num[x] = 16; }
else if (user_input[x] == 'Q' || user_input[x] == 'q') { corres_num[x] = 17; }
else if (user_input[x] == 'R' || user_input[x] == 'r') { corres_num[x] = 18; }
else if (user_input[x] == 'S' || user_input[x] == 's') { corres_num[x] = 19; }
else if (user_input[x] == 'T' || user_input[x] == 't') { corres_num[x] = 20; }
else if (user_input[x] == 'U' || user_input[x] == 'u') { corres_num[x] = 21; }
else if (user_input[x] == 'V' || user_input[x] == 'v') { corres_num[x] = 22; }
else if (user_input[x] == 'W' || user_input[x] == 'w') { corres_num[x] = 23; }
else if (user_input[x] == 'X' || user_input[x] == 'x') { corres_num[x] = 24; }
else if (user_input[x] == 'Y' || user_input[x] == 'y') { corres_num[x] = 25; }
else if (user_input[x] == 'Z' || user_input[x] == 'z') { corres_num[x] = 26; }
else { output_str_assign = user_input[x]; }//keeping user input intact if user input is not within a-z
calc = corres_num[x] + cipher_value;//assigning <calc> value
if (calc > 26)//setting value if <calc> goes over 26
{
    output = calc - 26;
}
else if (calc <= 26 && calc > 0)//maintaing <calc> value if within range
{
    output = calc;
}
switch (output)//assigning individual cipher characters
{
case 1:if (isupper(user_input[x])) { output_str_assign = "A"; }
      else if (islower(user_input[x])) { output_str_assign = "a"; } break;
case 2:if (isupper(user_input[x])) { output_str_assign = "B"; }
      else if (islower(user_input[x])) { output_str_assign = "b"; } break;
case 3:if (isupper(user_input[x])) { output_str_assign = "C"; }
      else if (islower(user_input[x])) { output_str_assign = "c"; } break;
case 4:if (isupper(user_input[x])) { output_str_assign = "D"; }
      else if (islower(user_input[x])) { output_str_assign = "d"; } break;
case 5:if (isupper(user_input[x])) { output_str_assign = "E"; }
      else if (islower(user_input[x])) { output_str_assign = "e"; } break;
case 6:if (isupper(user_input[x])) { output_str_assign = "F"; }
      else if (islower(user_input[x])) { output_str_assign = "f"; } break;
case 7:if (isupper(user_input[x])) { output_str_assign = "G"; }
      else if (islower(user_input[x])) { output_str_assign = "g"; } break;
case 8:if (isupper(user_input[x])) { output_str_assign = "H"; }
      else if (islower(user_input[x])) { output_str_assign = "h"; } break;
case 9:if (isupper(user_input[x])) { output_str_assign = "I"; }
      else if (islower(user_input[x])) { output_str_assign = "i"; } break;
case 10:if (isupper(user_input[x])) { output_str_assign = "J"; }
       else if (islower(user_input[x])) { output_str_assign = "j"; } break;
case 11:if (isupper(user_input[x])) { output_str_assign = "K"; }
       else if (islower(user_input[x])) { output_str_assign = "k"; } break;
case 12:if (isupper(user_input[x])) { output_str_assign = "L"; }
       else if (islower(user_input[x])) { output_str_assign = "l"; } break;
case 13:if (isupper(user_input[x])) { output_str_assign = "M"; }
       else if (islower(user_input[x])) { output_str_assign = "m"; } break;
case 14:if (isupper(user_input[x])) { output_str_assign = "N"; }
       else if (islower(user_input[x])) { output_str_assign = "n"; } break;
case 15:if (isupper(user_input[x])) { output_str_assign = "O"; }
       else if (islower(user_input[x])) { output_str_assign = "o"; } break;
case 16:if (isupper(user_input[x])) { output_str_assign = "P"; }
       else if (islower(user_input[x])) { output_str_assign = "p"; } break;
case 17:if (isupper(user_input[x])) { output_str_assign = "Q"; }
       else if (islower(user_input[x])) { output_str_assign = "q"; } break;
case 18:if (isupper(user_input[x])) { output_str_assign = "R"; }
       else if (islower(user_input[x])) { output_str_assign = "r"; } break;
case 19:if (isupper(user_input[x])) { output_str_assign = "S"; }
       else if (islower(user_input[x])) { output_str_assign = "s"; } break;
case 20:if (isupper(user_input[x])) { output_str_assign = "T"; }
       else if (islower(user_input[x])) { output_str_assign = "t"; } break;
case 21:if (isupper(user_input[x])) { output_str_assign = "U"; }
       else if (islower(user_input[x])) { output_str_assign = "u"; } break;
case 22:if (isupper(user_input[x])) { output_str_assign = "V"; }
       else if (islower(user_input[x])) { output_str_assign = "v"; } break;
case 23:if (isupper(user_input[x])) { output_str_assign = "W"; }
       else if (islower(user_input[x])) { output_str_assign = "w"; } break;
case 24:if (isupper(user_input[x])) { output_str_assign = "X"; }
       else if (islower(user_input[x])) { output_str_assign = "x"; } break;
case 25:if (isupper(user_input[x])) { output_str_assign = "Y"; }
       else if (islower(user_input[x])) { output_str_assign = "y"; } break;
case 26:if (isupper(user_input[x])) { output_str_assign = "Z"; }
       else if (islower(user_input[x])) { output_str_assign = "z"; } break;
}
output_str += output_str_assign;//adding to output ciphertext
}
int main()
{
bool run = true;//run validator
char run_again;//user choice for run again
while (run)//will run while <run> is true
{
    std::cout << std::endl;
    std::cout << " Cipher value (rot[2-25]): ";
    std::cin >> cipher_value;//user input
    while (cipher_value > 25 || cipher_value < 2)//checking illegal input 
    {
        std::cout << " Invalid...cipher value can only be between 2-25: ";
        std::cin >> cipher_value;//user input
    }
    std::cin.ignore(1000, '\n');
    std::cout << std::endl << std::endl;
    std::cout << " Input====> ";
    std::getline(std::cin, user_input);//user input
    for (x = 0; x < user_input.size(); x++)
    {
        user_input[x];
        convertion();//calling string convertion
    }
    std::cout << " Output===> " << output_str << std::endl;
    std::cout << std::endl << std::endl;
    std::cout << " Press any key to continue . . .";
    _getwch();
    std::cout << std::endl;
    termclr();//clear screen
    std::cout << std::endl;
    std::cout << " Run again? (Y/N): ";
    run_again = toupper(_getwche());//user input
    std::cout << std::endl;
    while (run_again != 'Y' && run_again != 'N')//checking illegal input
    {
        std::cout << " Invalid...try again (Y/N): ";
        run_again = toupper(_getwche());//user input
        std::cout << std::endl;
    }
    termclr();//clear screen
    output_str = "";
    if (run_again == 'Y')
    {
        run = true;//will run again
    }
    else if (run_again == 'N')
    {
        run = false;//wont run again
    }
}
return 0;
}

Answer 1

std::string user_input;//user input string

In C++ you don't need to declare variables before you use them. I would strongly recommend doing so, otherwise readers will have problems with the text. You certainly should not use namespace / global variables unless they are required - and here they are certainly not. Use method parameters and local variables instead.

Furthermore, the comment is entirely unnecessary. Commenting is good, but only if it adds information to the information available from the code / identifier names themselves.

int corres_num[1500000] = {};//corresponding number according to alphabets

This doesn't mean a thing to me. Corresponding to what? Why is it this number?

It is generally also a bad idea to assign default values (in this case the empty array).

system("clear");

Always try and avoid system dependent calls. Rather use a console library to perform the work; consoles have escape codes to perform clearance where required. Or simply don't clear the screen.

void convertion()//user input convertion

Here you do add information to the code, but why not just use convert_user_input? Methods should have verbs as names as they are doing things.

if (user_input[x] == 'A' || user_input[x] == 'a') { corres_num[x] = 1; }
...
else if (user_input[x] == 'Z' || user_input[x] == 'z') { corres_num[x] = 26; }

Real programmers don't use 1-based indexing. Real programmers do use 0-based indexing.

Gets repetitive doesn't it? Can you think of another way to convert a character to an index? Maybe the character is already assigned a number value and you can convert that to the index? And maybe that should be put in a function of itself?

If you ever need long if ... else lines (not here) then remember the switch and case keywords.

Note that it is a good thing that you convert to an index before doing the calculations!

else { output_str_assign = user_input[x]; }//keeping user input intact if user input is not within a-z

That comment would be perfect for a comment of a function.

calc = corres_num[x] + cipher_value;//assigning <calc> value
if (calc > 26)//setting value if <calc> goes over 26
{
    output = calc - 26;
}
else if (calc <= 26 && calc > 0)//maintaing <calc> value if within range
{
    output = calc;
}

Here you might want to look up how to perform modular calculations. Again, this might be put into another function. Programming is all about splitting up your problem into smaller problems (and putting those together in a well structured way).

switch (output)//assigning individual cipher characters

Ah, you do know about switch. However, this is still better calculated. Hint: I generally use things like index + 'A' or index + 'a'.

case 1:if (isupper(user_input[x])) { output_str_assign = "A"; }

Maybe you can simply remember the case instead. So a boolean is_lower_case would be perfect here. Now you have multiple checks for the same thing.

std::cout << " Cipher value (rot[2-25]): ";

What's exactly wrong with rot1?

run_again = toupper(_getwche());//user input

Try not to mix high level (std::cout and low level code (_getwche()) too much. What about std::cin?

---

To give you an idea on how to code this, here's some pseudo code for Caesar cipher:

caesar_encrypt(text, key)
{
    ciphertext = empty
    for (each char in text)
    {
        if (char is not in alphabet)
            add char to ciphertext
            continue; // quickly exit the current scope, it is handled now

        index = char_to_index(char)
        newIndex = index + key (modulus the size of the alphabet)
        newChar = index_to_char(newIndex)
        add char to ciphertext
    }
    return ciphertext
}

The input / output handling goes into the main function.

Answer 2

Far too many global variables - a simple text filter shouldn't need any.

There's a complete lack of checking that input operations succeeded. I would expect a few more if (!std::cin) tests to catch invalid user input.

Misspelt std::system() and failed to include <cstdlib> that declares it.

Also failed to include <cctype> for declaration of std::isupper() and std::islower() (which are also misspelt throughout).

No declaration for non-standard function _getwche() - if that's something you wrote, change its name to one you're allowed to use (not beginning with _).

When using the functions in <cctype>, remember that they take the unsigned value of a character in an int variable, so plain char needs to be converted to unsigned char before promotion.

This code is very verbose:

if (run_again == 'Y')
{
    run = true;//will run again
}
else if (run_again == 'N')
{
    run = false;//wont run again
}

Given that we've already limited run_again to those two values, a concise equivalent is simply

run = run_again == 'Y';

This is inefficient:

std::cout << std::endl << std::endl;

There's really no need to flush the stream twice like that. Ordinary newlines would be a better choice (for both).

Answer 3

Please keep in mind that I'm still new to programming and don't know advanced level stuff.

Important lesson: Decompose a problem using top down decomposition. Meanwhile, each function should do one thing.

You are separating the I/O and user interface logic from the main problem OK, by putting your actual cypher in conversion. But that function is doing too many different things. Clearly you can break it out into major pieces, which you should put into separate named functions rather than just flowing from one to another.

Lesson: functions should take input in the form of parameters, and return results via its return value.

Your conversion function just operates on fixed global variables. You should pass in the string to operate on, and return the result. Try writing it with no global variables at all.

---

Your huge blocks of code to convert between numbers and letters should raise a red flag. This should be built into any good programming language; typically they would be called chr and ord or something like that.

But in C++, which comes from C, it is implicit. A character is a small integer. You don't need a separate named function, you just cast (or implicitly convert) between the types.

In the vast majority of environments, the source character set will be a superset of ASCII, in which the codes for the letters a through z are consecutive; likewise for A through Z. If you want to be portable to exotic systems with source code saved in a strange character set, as of C++17 you can write a character literal with the u8 prefix to explicitly specify that it's an ASCII value (One-byte UTF-8 characters are ASCII).^※

If you had broken out your code to

• convert character to numeric value
• do the arithmetic
• convert number to character

as separate functions, then even if you wrote them out the hard way like you did in the OP, you could easily just replace the implementation of the first and third function with one-liners.

You're case-folding when doing the ord step, but then referring back to the input to see whether it was upper or lower case when doing the chr step. You should not have dependency between the old and new data like that; rather, flow cleanly from one step to the next.

To decompose the problem down to the most basic form, consider encoding a single capital letter. Write this as a function that takes and returns a value:

char encypher_one_capital_letter (char c, int CeasarShift=3)
{
    int n = c-u8'A';  // A becomes 0, B=1, ... Z=25
    n += CeasarShift;
    if (n >= 26)  n -= 26;  // roll over
    else if (n < 0)  n += 26;  // allow for negative shifts and easily use the same code to decode
    return char(n+u8'A');
}

Notice that the amount to shift was also passed in as a parameter. The code needs this value, so where does it come from? It should be supplied by the caller, as a parameter.

Now, given this (which you can try and TEST BY ITSELF and make sure it all works OK before you continue), you want to handle upper and lower case letters by preserving case, and pass through everything else unchanged.

That's another step of responsibility, and can be built on top of the more primitive function above.

char encypher_one_character (char c, int CeasarShift=3)
{
    if(!isletter(c))  return c;  // pass through things that are not letters
    ⋮

note that the "negative" is tested, and decisive action taken. That is, put simple things first. Now the rest of the function can proceed under the assumption that it is a letter.

    ⋮
    if (isupper(c))  return encypher_one_capital_letter(c, CeasarShift);
    // must be an lower-case letter:  preserve the case
    return to_lower(encypher_one_capital_letter(to_upper(c), CeasarShift));
}

Again, you can test this with examples of each case (pun intended) and make sure all is correct before continuing.

aside: what is to_upper and to_lower? Are those supplied library functions or something you need to write also? Either way, just assume that exists to do top-down decomposition.

Next, you want to process an entire string.

The best way to take general string input is using a std::string_view (by value). This will accept a std::string without having to copy anything, and also takes a lexical literal string "like this" efficiently, again without having to copy those chars into a std::string object.

string encypher (string_view input, int CeasarShift=3)
{
    ⋮

This needs to iterate over each input character and append an output character. You should expect that expressing this ought to be a very simple thing to do. If it seems complex, look for the right tool for the job. Going through all the items in a collection is a fundamental and very common thing to do.

    ⋮
    string result;
    for (char c : input) {
        char f = encypher_one_character(c,CeasarShift));
        result.push_back(f);
        // BTW, the output is called f because that is c encyphered 😉
    }
    return result;
}

Now consider the testing program for this final step: you don't want to have to manually type each test case each time you try it! Rather than getting input from the user, have automated test cases.

Now the final program. Looping and asking "Y/N to enter another" and prompting for input is busywork that is not part of this problem. And is that really what you want in a useful program? The Unix tools never do that! Just read from standard input (no prompts) and process each line, and stop when you hit EOF.

You can test it by redirecting input from a file, and enter text manually if you wanted to do that, using ctrl-d (on Unix; ctrl-z on Windows) when done.

> ./mycypherprogram 3 <inputs.txt

---

Good luck, and keep at it.

Footnote

※ Though you'll have other problems to worry about. When I was an intern at IBM, it was rather difficult to discuss C++ on the internal mailing list because the character set did not even have square bracket ([ and ]) characters! That is why C has trigraphs.

about your original code

for (x = 0; x < user_input.size(); x++)
    {
        user_input[x];
        convertion();//calling string convertion
    }

The line user_input[x]; doesn't do anything!
Inside the convertion [sic] you are stashing an intermediate value in corres_num[x] but only use the one cell of the array. There is no need to have an array of corres_num, since this pass of the function only ever looks at one character.

I think you got confused as to how the program worked, trying to do too much all at once. By doing top-down decomposition you can focus on one responsibility at a time. Learning that is more important than any single program. By implementing the simplified essence first, you simply did not have arrays to worry about! One char in, one char out, as if that's the only thing in the world. You had only the information you needed, and a single (simple) responsibility to perform. Thus, the code is simple and readable. By doing bottom-up implementation, you can test as you go: make sure that simplified thing works, then add the layer around it.