need help on debugging my c++ programming (text ascii manupulation)

Question

I recently made a C++ program which encrypts texts based-on the vigenere cipher technique.

The encryption part works fine I think, but the decryption function doesn't seem to output the correct answer in some cases. I was hoping if anyone could have a look at code and tell me what's wrong with it.

CMD dialog:

Enter the key for encrytion: magic

Enter message No. 1: I love C programming

Message encrypted: u rwxq i rdomzcymovi

Decrypted: i love c pXogramming

Somehow it outputted "X" instead of "r" in this case.............

Here are the codes:

Secret.h:

#ifndef CPP_TUTORIALS_SECRET_H
#define CPP_TUTORIALS_SECRET_H
#include <iostream>
using namespace std;

class Secret {

private:
    string message;
    string key;
    bool toEncrypt;
    bool toDecrypt;

public:
    bool isToDecrypt() const {
        return toDecrypt;
    }

    Secret(const string &message = "", const string &key = "", bool toEncrypt = false);

    ~Secret();

    void setToDecrypt(bool toDecrypt);

    void encrypt();

    void decrypt();

    void display();

};

#endif //CPP_TUTORIALS_SECRET_H

Secret.cpp

#include "Secret.h"

Secret::Secret(const string &message, const string &key, bool toEncrypt) {
    Secret::message = message;
    Secret::key = key;
    Secret::toEncrypt = toEncrypt;
}

Secret::~Secret(){

}

void Secret::setToDecrypt(bool toDecrypt) {
    Secret::toDecrypt = toDecrypt;
}

void Secret::display() {
    cout << message << endl;
}

void Secret::encrypt() {
    if(toEncrypt) {
        int keyAscii[key.length()];
        int count = 0;

        for(unsigned int i = 0; i < key.length(); i++) {
            keyAscii[i] = key.at(i);
        }

        for(unsigned int i = 0; i < message.length(); i++) {
            if (message.at(i) > 64 && message.at(i) < 91) {
                message.at(i) = (char)((message.at(i) - 65 + (keyAscii[count] - 97)) % 26 + 97);
            }
            else if (message.at(i) > 96 && message.at(i) < 123) {
                message.at(i) = (char)((message.at(i) - 97 + (keyAscii[count] - 97)) % 26 + 97);
            }
            else{
                message.at(i) = message.at(i);
            }
            count++;
            if(count == key.length()) {
                count = 0;
            }
        }
    }
}

void Secret::decrypt() {
    if(toDecrypt) {
        int keyAscii[key.length()];
        int count = 0;

        for(unsigned int i = 0; i < key.length(); i++) {
            keyAscii[i] = key.at(i);
        }

        for(unsigned int i = 0; i < message.length(); i++) {
            if (message.at(i) > 96 && message.at(i) < 123) {
                message.at(i) = (char)((message.at(i) - 97) % 26 - (keyAscii[count] - 97) + 97);
            }
            else {
                message.at(i) = message.at(i);
            }
            count++;
            if (count == key.length()) {
                count = 0;
            }
        }
    }
}

main.cpp

#include <limits>
#include "Secret.h"

void calcMsgAmount(int &pArraySize);
void inputKey(string &key);
void encryptMsg(Secret &secret, string &msg, const string &key, bool toEncrypt, int index);

int main() {
    int arraySize;
    string key;
    string msg;

    calcMsgAmount(arraySize);
    inputKey(key);

    Secret secrets[arraySize];

    for(int i = 0; i < arraySize; i++){
        encryptMsg(secrets[i], msg, key, true, i);
    }

    cout << endl << "Message encrypted: " << endl;
    for(Secret i: secrets){
        i.display();
        i.setToDecrypt(true);
        if(i.isToDecrypt()){
            i.decrypt();
            cout << endl << "Decrypted: " << endl;
            i.display();
        }
        cout << endl << endl;
    }

    return 0;
}

void calcMsgAmount(int &pArraySize) {
    cout << "Enter the amount of messages you want to input: " << endl;
    while(!(cin >> pArraySize)){
        cout << endl << "There's something really wrong with your input, please enter again." << endl;
        cout << "Enter the amount of messages you want to input: " << endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
}

void inputKey(string &key){
    cout << "Enter the key for encrytion: " << endl;
    cin.ignore();
    getline(cin, key);
}

void encryptMsg(Secret &secret, string &msg, const string &key, bool toEncrypt, int index){
    cout << "Enter message No. " << index + 1 << ": " << endl;
    getline(cin, msg);
    secret = Secret(msg, key, toEncrypt);
    secret.encrypt();
}

Many thanks

Have you tried any debugging yourself? Did you try to run in an actual debugger, and step through line by line to see that it works as expected? — Some programmer dude, Aug 02 '15 at 14:55
And please don't use ["magic numbers"](https://en.wikipedia.org/wiki/Magic_number_%28programming%29#Unnamed_numerical_constants), use the appropriate character literals instead, like e.g. `'a'` instead of `97`. Or better yet, use the proper [`std::isupper`](http://en.cppreference.com/w/cpp/string/byte/isupper) and [`std::islower`](http://en.cppreference.com/w/cpp/string/byte/islower) functions when appropriate. — Some programmer dude, Aug 02 '15 at 14:56
@JoachimPileborg I did, I tried with other texts as well, somehow it worked for some characters in the sentence but others don't work. — Jay Feng, Aug 02 '15 at 15:00
The nice things about debugger is that it's easy to check the values of variables, and if you're using a debugger connected to an IDE then it's even *very* easy. When stepping through the code, for each step check the values of all involved variables, and see if they take on the correct value. If you suddenly get a value you should not get, then you narrowed it down to the problematic statement, and can start examining it closer to see why it produced the wrong value. — Some programmer dude, Aug 02 '15 at 15:03
I am new to C++, but I would also like to ask, should I use "using namespace std" for my programs? It seems make me easier because I don't need to type std:: everytime I need to use it. — Jay Feng, Aug 02 '15 at 15:04
In the `encrypt` function and in `main` you're using variable length arrays, which are a C99 feature. Please stick to one language only. — Banex, Aug 02 '15 at 15:10
Read e.g. [Why is “using namespace std;” considered bad practice?](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Some programmer dude, Aug 02 '15 at 15:13
@JayFeng You forgot to include ``. `It seems make me easier because I don't need to type std:: everytime`. And it is easier to "clean a room" by sweeping the dirt under the rug. — PaulMcKenzie, Aug 02 '15 at 15:14
@JayFeng You can pull only the stuff you need into the global namespace with, for example, `using std::cout` and `using std::endl. Not as dangerous as pulling in a very, very large and diverse library, but still requires some care and attention. — user4581301, Aug 02 '15 at 15:55
@Banex Thanks for pointing me out, I am really new to c++ so there are a lot of things that I am unaware of, I should go look it up and see the differences between the C++99 and C++11 — Jay Feng, Aug 02 '15 at 22:19

Effie · Answer 1 · 2015-08-02T15:32:20.267

accroding to wikipedia the decrypt routine of Vigenere Cipher is

(C - K) mod 26

what you wrote in Secret::decrypt() is

message.at(i) = (char)((message.at(i) - 97) % 26 - (keyAscii[count] - 97) + 97);

which is C % 26 - K.

I changed the line to

message.at(i) = (char)((26 + (message.at(i) - 97) - (keyAscii[count] - 97)) % 26 + 97);

it seems to be correct. I haven't really understood why the first 26 is necessary, but without it the code doesn't work (something with % and negative numbers)

P.S. as for debugging part, you might have noticed that whong letters appear as wrong capital letters which have smaller ascii code, so you decrypt routine has negative numbers in it. After which you check your code against Wikipedia :)

Christophe · Accepted Answer · 2015-08-02T16:08:50.987

The problem comes in the decrypt from two problems:

the first is that the calculation should be symetric from the the encryp, i.e. the modulo 26 for message - key (as already pointed out by effenok)
the second is that in the decrypt you can at char level, message-key can be negative, which might not give you the expected results modulo 26 (for example -2 % 26 is -2 so that you'll be out of the alphabetic range that you expect). The easy trick is to add 26 before doing the modulo, which makes sure it's done on a positive number.

Here slightly more compact functions, making the difference between uppercase and lowercase :

void Secret::encrypt() {
    if(toEncrypt) {
        for(unsigned int i = 0; i < message.length(); i++) {
            if(isalpha(message[i])){
                char start = isupper(message[i]) ? 'A' : 'a';
                message[i] = (message[i] - start + (tolower(key[i%key.length()]) - 'a') + 26) % 26 + start;
            }
        }
    }
}
void Secret::decrypt() {
    if(toDecrypt) {
        for(unsigned int i = 0; i < message.length(); i++) {
            if (isalpha(message[i]) ){
                char start = isupper(message[i]) ? 'A':'a'; 
                message[i] = (message[i] - start - (tolower(key[i%key.length()]) - 'a') + 26) % 26 + start;
            }
        }
    }
}

@Cristophe: is `a % b` defined in C as `sign(a) * ( abs(a) % b)` ? — Effie, Aug 02 '15 at 15:49
@effnok exactly, this is why the decrypt was out of the expected range. — Christophe, Aug 02 '15 at 15:52

need help on debugging my c++ programming (text ascii manupulation)

2 Answers2