Thread: Crashing Vigenere Cipher program

I made a program to encode and decode messages using a Vigenere Cipher and a key. (if you dont know what a Vigenere Cipher is... then check Wikipedia.) It should work *fine*, but it keeps hanging at the point when it should show the encrypted message/decrypted message.

Code:

#include<iostream>
#include<windows.h>
using namespace std;
class Frame
{
 public:
        char square();
        int enc();
        int dec();
};
char Frame::square()
{
    // ===(Vigenere Square Generator)===
    char a[26][26], alpha='a', flag='a', showvig;
    for(int col=0; col<26; col++)
    {
            for(int row=0; row<26; row++)
            {
                    a[col][row]=alpha;               //'a[][]' is assigned alpha's value
                    alpha++;                         //Alpha increased
                    if(alpha>'z')
                                 alpha='a';          //continues the series after z
//                  cout<<a[col][row]<<" ";          //optional display statement
            }
            flag++;                                  //Value of flag increased
            alpha=flag;                              //Value of flag assigned to a
    }
/*    cout<<"\nDisplay Vigenere Square?(Y/N) ";
    cin>>showvig;
    if(showvig=='Y' || showvig=='y')
    {
        for(int col=0; col<26; col++)
        {
            for(int row=0; row<26; row++)
            {
                cout<<a[col][row]<<" ";
            }
            cout<<endl;
        }
    } */
    return a[26][26];   
}
int Frame::enc()
{   
    char b[26][26];
    b[26][26]=square();
    // ===(ACME Encrypt O Matic)===
    char key[50], srchk, srchm, message[100], encoded[100];
    int keylen, msglen, arow, bcol, k;
    cout<<"\nEncrypter ready";
    cout<<"\nInsert key... ";
    cin.getline(key,50);
    cin.getline(key,50);
    cout<<"\nShowing the key..... "<<key<<key<<key;  //Comparison purposes
    cout<<"\nEnter the message... ";
    cin.getline(message,100);
    keylen=strlen(key);
    msglen=strlen(message);
    cout<<"\nEncrypted message... ";
    for(int i=0; i<msglen; i++)
    {
            k=i;                                    //Allows for the messages
            if(i>keylen)                            //to be longer than the
                        k=0;                        //key ie repeating key behaviour
            srchk=key[k];
            srchm=message[i];
            if(message[i]==' ')
            {
                        encoded[i]=' ';
                        goto label;
            }
            for(int col=0, row=0; col<26; col++)
            {
                    if(srchk==b[row][col])
                    {
                                          bcol=col;
                                          break;
                    }
            }
            for(int row=0, col=0; row<26; row++)
            {
                       if(srchm==b[row][col])
                       {
                                             arow=row;
                                             break;
                       }
            }
            encoded[i]=b[arow][bcol];
            label:
            cout<<encoded[i];
    }
    cout<<endl;
    system("PAUSE");
    return 0;
}
int Frame::dec()
{    
     char c[26][26];
     c[26][26]=square();
    // ===(ACME Encryption-Be-Gone)===
     char key[50], srchk, srchm, message[100], encoded[100];
     int keylen, msglen, arow, bcol, k;
     cout<<"\nInitializing...";
     Sleep(5);
     cout<<"\nDecrypter ready";
     cout<<"\nInsert key... ";
     cin.getline(key,50);
     cin.getline(key,50);
     cout<<"\nShowing the key..... "<<key<<key<<key;  //Comparison purposes
     cout<<"\nEnter coded mssg.... ";
     cin.getline(encoded,100);
     keylen=strlen(key);
     msglen=strlen(encoded);
     cout<<"\nDecrypted message... ";
     for(int i=0; i<msglen; i++)
     {
             k=i;                                    //Allows for the messages
             if(i>keylen)                            //to be longer than the
                         k=0;                        //key ie repeating key behaviour
             srchk=key[k];
             srchm=encoded[i];
             if(encoded[i]==' ')
             {
                         message[i]=' ';
                         goto label2;
             }
             for(int col=0, row=0; col<26; col++)
             {
                     if(srchk==c[row][col])
                     {
                                           bcol=col;
                                           break;
                     }
             }
             for(int row=0, col=bcol; row<26; row++) //col=0 changed
             {
                        if(srchm==c[row][col])
                        {
                                col=0;
                                bcol=col;
                                arow=row;
                                break;
                        }
             }
             message[i]=c[arow][bcol];
             label2:
             cout<<message[i];
     }
     cout<<endl;
     system("PAUSE");
     return 0;
}
int main()
{
    Frame ob;
    int ch;
    cout<<"\n(1)Encrypt or (2)decrypt?";
    cin>>ch;
    if(ch==1)
             ob.enc();
    else
        if(ch==2)
                 ob.dec();
    return 0;
}

What exactly is t3h problem? It worked fine when I ran the encrypt/decrypt sections separately (by putting all the code in just the main(), and putting comments on the encrypt or decrypt sections)

-regards,
ultrabot90

It seems to me (after trying out your code) that you use variables that are undefined, e.g.:

Code:

			encoded[i]=b[arow][bcol];

arow is some huge value (Perhaps I'm doing something wrong in using the code tho').

I'm also pretty sure that you need to do this differently:

Code:

		k=i;                                    //Allows for the messages
		if(i>keylen)                            //to be longer than the
			k=0;                        //key ie repeating key behaviour
		srchk=key[k];

If you give "KEY" as the key, then after the third letter, it will just repeat "KKKKKKKKKKKKK" until the end of the message, which is of course not particularly good encryption (could just as well use cesars cipher in that case).

You probably want to do:

Code:

		k++;                                    //Allows for the messages
		if(k>keylen)                            //to be longer than the
			k=0;                        //key ie repeating key behaviour
		srchk=key[k];

--
Mats

Also

Code:

	return a[26][26];

That returns the 27th column of the 27th row! - You only have 26 of each.

Of course, looking further at your code, it seems like you actaully want to return the FULL 2D array.

--
Mats

@Daved - That indeed seems to be the problem...I've edited it, though I dunno if I did it right...

Code:

 public:
        void square();
        int enc(char[26][26]);
        int dec(char[26][26]);
};

Code:

void Frame::square()
{
    // ===(Vigenere Square Generator)===
    char a[26][26], alpha='a', flag='a', showvig;

Code:

int Frame::enc(char b[26][26])
{   
    // ===(ACME Encrypt O Matic)===
    char key[50], srchk, srchm, message[100], encoded[100];

Code:

int Frame::dec(char b[26][26])
{    
     // ===(ACME Encryption-Be-Gone)===

Code:

int main()
{
    Frame ob;
    int ch;
    cout<<"\n(1)Encrypt or (2)decrypt?";
    cin>>ch;
    if(ch==1)
             ob.enc(char[][]);
    else
        if(ch==2)
                 ob.dec(char[][]);
    return 0;
}

In the main(), however, it gives errors for both of the enc() and dec() functions, that "expected primary-expression before "char"". Now what do I do?

@matsp- Omg no, that part is correct, 100%! I've already said that it works fine when running individual encode/decode functions.
On the key repeating lines...
The key is supposed to be written on top of the message, again and again, so that every letter of the message matches up with a key letter it is to be encoded with. Hence, both key and message run with [i], simultaneously, on untill the message to be encoded/decoded ends (in key[k] I made an exception, lets see why ->). As soon as i becomes greater than the key length, k resets to 0, and since key is working according to k, it repeats...Simple! ^_^

arow and bcol are just used to store the 'coordinates' of the required character which is to be input into the encoded/decoded message. arow and bcol arent huge, they're both less than 26 xD
EDIT - Omg the second thing hell thats true it'd return a[26][26]th element! Sheesh...
-regards,
ultrabot90

Why not leave the array inside the class as a private member variable, and save all this messing about passing the array in and out every time you want to do something.

And can I just say "ugh" to the goto's.

Gotos should go... Especially since the goto is fairly easy to get rid of by just using an "else" at the end of the if which contaisn the goto (and of course braces around the code down to the line before the now defunct label).

I also agree on the use of square as a member of the class.

By the way, your code only works right if the key and message are lower-case. Maybe you want to add

Code:

		srchk=tolower(key[k]);

and corresponding on srchm.

If you add the red bit:

Code:

	cin>>ch;
	cin.ignore();

you don't have to do two getline's (because the "ignore" bit will "eat" the newline that is otherwis sitting there and making key an empty string.

Of course, it's all broken for non-alpha characters, so perhaps your lines

Code:

		if(encoded[i]==' ')
		{
			message[i]=' ';

should be:

Code:

		if(!isalpha(encoded[i]))
		{
			message[i]= encoded[i];

And of course the corresponding code in encode.

The for-loops that you use for searching in the encode can be easily replaced with simple math:

Code:

    bcol = srchk - 'a';
    arow = srchm - 'a';

.
And the bcol can be done the same way in the decode part.


--
Mats

Originally Posted by Salem

Why not leave the array inside the class as a private member variable, and save all this messing about passing the array in and out every time you want to do something.

Yeah, that's something I would also suggest, have the table exist as a private data member. And it looks like you recreate the table each time you encrypt/decrypt something. Do it once (put the code that builds the table in the constructor for example) and access that table directly from the enc and dec functions, no need to pass it around anymore. You can ditch the square function then as well.

Code:

class Frame
{
    char a[26][26];
 public:
    //char square();  // Ditch this
    Frame();  // Add this
    int enc();
    int dec();
};

Frame::Frame()  // Builds the table
{
    for( int col=0; col<26; ++col )
        for( int row=0; row<26; ++row )
            a[col][row] = 'a' + ((col+row)%26);
}

Then you can get rid of the b and c arrays in enc and dec and just use the a array everywhere instead.

Doesn't crash on my machine. What do you do to crash it?

--
Mats

It ran, but with decryption errors...
With key as "systemofadown" it encrypts "omfg wtf" as "gkxz ihk"...but decrypts the same as "yips uvp".

Yes, you are doing "encryption" in the decryption part. Unfortunately, [I don't think, at least] you can't remove the loop bit out of the decode part.

Replacing the

Code:

arow = srchm - 'a'

with:

Code:

				for(int row=0; row<26; row++) //col=0 changed
				{
							if(srchm==a[row][bcol])
							{
									arow=row;
									break;
							}
				}

gives this:

Code:

(1)Encrypt or (2)Decrypt?1

Encrypter ready
Insert key... hello

Showing the key..... hellohellohello
Enter the message... abc

Encrypted message... hfn

(1)Encrypt or (2)Decrypt?2

Decrypter ready
Insert key... hello

Showing the key..... hellohellohello
Enter coded mssg.... hfn

Decrypted message... abc

--
Mats