0

I have been writing a very basic encryption program in C++ where the user is asked to enter a message, and then a number with the same number of digits as the number of characters in the message. Then it converts each character in the message to the ascii number, and then adds the each digit in the number to the ascii, and then converts back to a character and returns the encrypted string. And the progress happens in reverse to decrypt. When I tried to encrypt "hello", I got �����. Can someone please explain how I can fix this? I have not finished the code, but this is it so far:

#include <iostream>
using namespace std;

string encryptMsg(string msg)
{
    cout << "Enter a number that is " << msg.length() << " digits long: ";
    string codekey;
    cin >> codekey;
    char msgArr[msg.length()];
    int keyCode[msg.length()];
    int tempcount = 0;
    for(char& c : msg) {
        msgArr[tempcount] = c;
        tempcount++;
    };
    tempcount = 0;

    for(char& c : codekey) {
        keyCode[tempcount] = c;
        tempcount++;
    };
    double tempascii;
    for(int i = 0; i < (sizeof(msgArr)/sizeof(msgArr[0])); i++)
    {
        tempascii = msgArr[i];
        tempascii = tempascii + keyCode[i];
        msgArr[i] = tempascii;
    };
    string outmsg;
    for(int i = 0; i < (sizeof(msgArr)/sizeof(msgArr[0])); i++)
    {
        outmsg = outmsg + msgArr[i];
    };
    return outmsg;
}

string decryptMsg(string msg)
{
    char msgArr[msg.length()];
    int tempcount = 0;
    for(char& c : msg) {
        msgArr[tempcount] = c;
        tempcount++;
    };
    double tempascii;
    for(int i = 0; i < (sizeof(msgArr)/sizeof(msgArr[0])); i++)
    {
        tempascii = msgArr[i];
        tempascii = tempascii - 3;
        msgArr[i] = tempascii;
    };
    string outmsg;
    for(int i = 0; i < (sizeof(msgArr)/sizeof(msgArr[0])); i++)
    {
        outmsg = outmsg + msgArr[i];
    };
    return outmsg;
};

int main()
{
    string mode;
    string msg;
    cout << "Encryption Program" << endl;
    cout << "Would you like to encrypt or decrypt?" << endl;
    cin >> mode;
    if (mode == "encrypt" || mode == "Encrypt" || mode == "ENCRYPT" || mode == "1")
    {
        cout << "Encryption. ENTER MSG HERE: ";
        cin >> msg;
        cout << encryptMsg(msg);
    } else if (mode =="decrypt" || mode == "Decrypt" || mode == "DECRYPT" || mode == "2")
    {
        cout << "Decryption. ENTER MSG HERE: ";
        cin >> msg;
        cout << decryptMsg(msg);
    };
    return 0;
}

Thank you.

Beckett O'Brien
  • 176
  • 2
  • 16
  • 2
    `char msgArr[msg.length()];` -- This line and the one right after it are both invalid C++. Arrays in C++ must have the number of entries denoted by a compile-time expression, not a runtime value such as a variable or the result of a runtime function call. Use `std::vector msgArr(msg.length())` instead. Since those are not really arrays, I have no idea the result of doing things like this: `sizeof(msgArr)/sizeof(msgArr[0])`. With `vector`, you just use the `size()` function. – PaulMcKenzie Apr 02 '18 at 00:26
  • Also, you could have cut down your `main` function by just doing a direct call to encrypt or decrypt functions with known data, instead of posting all of that input/output code. – PaulMcKenzie Apr 02 '18 at 00:29
  • There's a lot of style issues with the code that I shall skip for now, but in `keyCode[tempcount] = c;` - do you mean `keyCode[tempcount] = c - '0';`? If the user types `9`, do you wish to add `57` or `9`? – Ken Y-N Apr 02 '18 at 00:30
  • You probably don't want to do the operations on the ASCII values, since you might end up with stuff like control characters. Instead, assign an integer value to each of the possible characters that might appear in the message. – eesiraed Apr 02 '18 at 01:08
  • Also, variable length arrays like `char msgArr[msg.length()];` aren't standard features, so it's a good idea not to use them, especially when you're also using `sizeof()`, which as far as I know is compile-time constant. I don't know why you're converting the strings into char arrays in the first place since you can just use the subscript operator (`[]`) on strings just like char arrays. – eesiraed Apr 02 '18 at 01:10
  • @FeiXiang That sounds like a very good idea and I will try it as soon as I get a chance. – Beckett O'Brien Apr 02 '18 at 01:37
  • @KenY-N Thank you, this fixed my problem, but I don't understand how. Why does it result in 57 if I type 9? – Beckett O'Brien Apr 02 '18 at 04:11
  • The ASCII code for '9' is 57, as you read in the digits as a string in `codekey`. You can see this by doing `std::cout << (int) '9' << '\n';` – Ken Y-N Apr 02 '18 at 04:14
  • @BeckettO'Brien visit [ASCII Table](http://www.asciitable.com/) and all will be clear. – David C. Rankin Apr 02 '18 at 04:33
  • @DavidC.Rankin Thank you. I understand now. I am not experienced with ASCII, and I am still confused why when I encrypt, I increase a value by 1, but the result is not as it would be on the ASCII table. – Beckett O'Brien Apr 02 '18 at 04:55
  • You had a number of problems in your code that were giving you grief. I put together an answer below to help you sort them out. – David C. Rankin Apr 02 '18 at 07:12

1 Answers1

0

You have a number of potential problems reading data with cin. If any of your input contains whitespace (intentional or unintentional), your code breaks. (they same occurs if you use a type that doesn't consume leading whitespace, such as char) Better to use line-oriented input such as getline to read user input to insure your '\n' generated by pressing Enter is read and discarded.

Your code lacks any meaningful validation. You must validate all user input to insure there were no stream errors encountered, the user didn't cancel input by generating a manual EOF with Ctrl+C (or Ctrl+Z on windows -- but see: CTRL+Z does not generate EOF in Windows 10)

Further, if you are reading digits into a string you will rely on for encryption, you need to validate that each character read into codekey was in fact a digit. (the functions in <cctype>, like isdigit() can help).

As explained in the comments char msgArr[msg.length()]; is wrong. The C++ standard does not include Variable Length Arrays (VLA's) like the C standard does. gcc offers VLA's as an extension to the standard but that makes your code non-portable.

If you simply want to encrypt the characters in msg in-place, there is no need for any other string or storage in encryptMsg -- just add your digit to the character in msg in place. For example, you could do something as simple as the following to encrypt msg in-place:

string encryptMsg(string& msg)
{
    string codekey;             /* string codekey, result */
    const char *k;              /* pointer to chars in codekey */

    cout << "Enter a number that is " << msg.length() << " digits long: ";
    if (!getline (cin, codekey)) {          /* read codekey */
        cerr << "error: invalid input - codekey\n";
        exit (EXIT_FAILURE);
    }
    if (codekey.length() < msg.length()) {  /* validate codekey length */
        cerr << "error: insufficient number of digits entered.\n";
        exit (EXIT_FAILURE);
    }
    k = codekey.c_str();        /* initialize k to 1st char in codekey */
    for (char& c : msg) {       /* loop over each char in msg */
        if (!isdigit(*k)) {     /* validate char is a digit */
            cerr << "error: non-numeric input '" << *k << "'\n";
            exit (EXIT_FAILURE);
        }
        c = c + (*k++ - '0');   /* add digit val to c, encrypting msg */
    }
    return msg;                 /* return encrypted result */
}

To preserve msg, you can simply declare outmsg as a string as well and assign outmsg = msg; and then encrypt the characters in outmsg with the digits entered in codekey, e.g.

string encryptMsg(string& msg)
{
    string codekey, outmsg = msg;   /* string codekey, outmsg */
    const char *k;                  /* pointer to chars in codekey */

    cout << "Enter a number that is " << msg.length() << " digits long: ";
    if (!getline (cin, codekey)) {          /* read codekey */
        cerr << "error: invalid input - codekey\n";
        exit (EXIT_FAILURE);
    }
    if (codekey.length() < msg.length()) {  /* validate codekey length */
        cerr << "error: insufficient number of digits entered.\n";
        exit (EXIT_FAILURE);
    }
    k = codekey.c_str();        /* initialize k to 1st char in codekey */
    for (char& c : outmsg) {    /* loop over each char in msg */
        if (!isdigit(*k)) {     /* validate char is a digit */
            cerr << "error: non-numeric input '" << *k << "'\n";
            exit (EXIT_FAILURE);
        }
        c += (*k++ - '0');   /* add digit val to c, encrypting msg */
    }
    return outmsg;           /* return encrypted outmsg */
}

Otherwise, C++ provides for simple allocation with new and delete, or you can simply use a string for codekey -- but then you will need an iterator to help iterate over the characters in codekey while simultaneously iterating over the characters in msg. It's up to you, but in these cases I find a simple allocation for the resulting message and using a simple character pointer to fill the encrypted string before assigning to a string to return is about as easy as anything else, e.g.

string encryptMsg(string& msg)
{
    char *tmp = new char[msg.length() + 1], /* allocate for length + 1 */
        *p = tmp;                           /* pointer to tmp */
    const char *k;                          /* pointer to use for key */
    string codekey, res;                    /* string codekey, result */

    cout << "Enter a number that is " << msg.length() << " digits long: ";
    if (!getline (cin, codekey)) {          /* read codekey */
        cerr << "error: invalid input - codekey\n";
        exit (EXIT_FAILURE);
    }
    if (codekey.length() < msg.length()) {  /* validate codekey length */
        cerr << "error: insufficient number of digits entered.\n";
        exit (EXIT_FAILURE);
    }
    k = codekey.c_str();        /* initialize k to 1st char in codekey */
    for (char& c : msg) {       /* loop over each char in msg */
        if (!isdigit(*k)) {     /* validate char is a digit */
            cerr << "error: non-numeric input '" << *k << "'\n";
            exit (EXIT_FAILURE);
        }
        *p++ = c + (*k++ - '0');    /* add digit val to c, store in tmp */
    }
    *p = 0;         /* nul-termiante tmp */
    res = tmp;      /* assign char array to string res */
    delete[] tmp;   /* free allocated memory */
    return res;     /* return encrypted result */
}

You can put a short example program together to validate the encrypt to insure it is doing what you intend it to do. Below, we enter "hello" as msg, encrypt and use "11111" as codekey to get the intended "ifmmp" returned by encryptMsg, e.g.

#include <iostream>
#include <string>
#include <cctype>
#include <limits>

using namespace std;

string encryptMsg(string& msg)
{ ... }

int main (void) {

    string mode, msg;

    cout << "mode: ";
    if (!getline (cin, mode)) { /* read mode (discarding trailing '\n') */
        cerr << "error: invalid input - mode\n";
        return 1;
    }
    for (char& c : mode)        /* convert mode to lowercase */
        c = tolower(c);
    if (mode == "encrypt" || mode == "1") { /* check mode */
        cout << "msg : ";           /* prompt for msg */
        if (!getline (cin, msg)) {  /* read msg */
            cerr << "error: invalid input - msg\n";
            return 1;
        }
        /* output encrypted result */
        cout << "res : " << encryptMsg(msg) << '\n';
    }
}

(note: all functions generate the same output, but in the final two msg is preserved unchanged)

Compile With Warnings Enabled

For all code you compile, you want to compile with Warnings Enabled, for gcc/g++ and clang, you can use -Wall -Wextra -pedantic for a fairly inclusive set of warnings, clang also offers -Weverything. VS (cl.exe) uses /W# where # is 1-4, it also offers /Wall (which really means all), and you can disable specific warnings you don't want reported with /w#### where #### is the code for the warning to disable. Do not accept code until it compiles without warning.

Read an understand any warnings you get, they point out specific problems and the line on which the problem can be found. (g++ goes a bit overboard in producing a plethora of messages, but still, they identify the problem and line). You can learn a lot by listening what your compiler is trying to tell you.

Example Use/Output

$ ./bin/asciiencrypt3
mode: EnCRypt
msg : hello
Enter a number that is 5 digits long: 11111
res : ifmmp

Since this looks a lot like an assignment, you can tackle the remainder of the implementation from here. If you run into problems, add your updated code below your existing question, and I'm happy to help further.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Thanks you! This helped a lot. I still have to add in safety for if they enter a number that does not correspond to anything on the ASCII chart, but I want to thank you for working with me even though I am very bad at c++ – Beckett O'Brien Apr 04 '18 at 01:09
  • When everybody starts learning c++, they are all very bad at it. It's supposed to feel difficult, just as learning anything worthwhile feels when you first start. Don't worry, if you work at it, and focus on learning to do it right -- not just making it work, you will be a wizard in no time. (well, a reasonable amount of time...) – David C. Rankin Apr 04 '18 at 02:00
  • Thank you for the advice, but if I may: do you know of any good online c or c++ courses? – Beckett O'Brien Apr 04 '18 at 21:26
  • [Definitive C++ Guide](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and always keep [C++ language](http://en.cppreference.com/w/cpp/language) handy, and while not flawless, working through the [cplusplus tutorial](http://www.cplusplus.com/doc/tutorial/) you will have a good primer for the basics. – David C. Rankin Apr 04 '18 at 21:59