Thread: Playfair Cipher program - toupper() function causing exceptions

As the title implies, I'm working on a playfair cipher program.

At the moment, I'm only trying to create the LxW 2D character array, using capital letters and the 0-9 digits and a user-inputted key, and get it to display. This being said...

I want to solve all of my logic flaws for myself. Please do not post things that have nothing to do with

I have no idea what is causing this problem, so I have included all code. The program does compile and whenever it breaks it points directly to the line in which the toupper function is written, but even after running once (giving me an incorrect cipher) and is closed, I end the program and re-run it without touching a single line of code, only for it to give me another exception.

All code is below, and the offending line is marked with a comment featuring *s.

Code:

#include <iostream>
#include "string"
using namespace std;


int main()
{
	char cipher[6][6], key[36], input[100], choice = 'N', c = '0';
	string result;
	int row, col, a, b;


	while (choice != 'Q')
	{
		if (choice == 'N') // If creating new cipher
		{
			// Clears key for new cipher creation
			for (int b = 0; b < 36; b++)
				key[b] = '\0';


			cout << "Using A-Z and/or 0-9, please enter a key for the playfair cipher (Use no more than 36 characters, duplicate letters will be deleted):  ";
			cin >> key;
			cout << endl << endl;


			// Converts all letters to upper case
			for (int c = 0; c < 36; ++c)
			{
                                // If character is not junk, capitalize it.
				if (key[c] != '\0')
					key[c] = toupper(static_cast<unsigned char>(key[c]));     // *****Problem is here*****
			}


			// Count involves Key length, co involves cipher col/row
			int count = 0, co = 0;


			// Create the cipher using Key
			while (count < 36)
			{
				if (key[count] != NULL)
				{
					while (c != key[count])
					{
						// Loops to current slot to see if letter is already included
						for (a = 0; a <= co; a++)
						{
							row = a / 6;
							col = a % 6;
							c = cipher[row][col];
						}


						// Inserts character
						row = co / 6;
						col = co % 6;
						cipher[row][col] = key[count];
						co++;
					}
				}
				// Insert 0-9
				for (char insert = '0'; insert <= '9'; insert++)
				{
					while (c != key[count])
					{
						// Loops to current slot to see if letter is already included
						for (a = 0; a <= co; a++)
						{
							row = a / 6;
							col = a % 6;
							c = cipher[row][col];
						}


						// Inserts character
						row = co / 6;
						col = co % 6;
						cipher[row][col] = key[count];
						co++;
					}
				}


				for (char insert = 'A'; insert <= 'Z'; insert++)
				{
					while (c != key[count])
					{
						// Loops to current slot to see if letter is already included
						for (int a = 0; a <= co; a++)
						{
							row = a / 6;
							col = a % 6;
							c = cipher[row][col];
						}


						// Inserts character
						row = co / 6;
						col = co % 6;
						cipher[row][col] = key[count];
						co++;
					}
				}


				count++;
			}
		}


		for (a = 0; a < 6; a++)
		{
			for (b = 0; b < 6; b++)
				cout << cipher[a][b] << " ";
			cout << endl;
		}


		cout << "If you would like to create a new cipher, enter N." << endl;
		cout << "If you would like to encode another sentence, enter E." << endl;
		cout << "If you would like to quit, enter Q." << endl;
		cout << "Please enter a character now:  ";
		cin >> choice;
		choice = (toupper(choice));


		while (choice != 'N' && choice != 'E' &&choice != 'Q')
		{
			cout << "ERROR:  Invalid choice.  Enter a valid character (N, E, or Q):  ";
			cin >> choice;
			choice = (toupper(choice));
		}
	}


	system("pause");
	return 0;
}

Originally Posted by algorism

You shouldn't use the cast on the input to toupper.

You should learn to use functions.

As an alternative algorithm for constructing the grid, consider using an array holding the string "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789". The index of a letter in the string is given by:

Code:

// 'A'=>0,'B'=>1,..,'Z'=>25,'0'=>26,..,'9'=>35
int get_index(char c) { // Assumes input: A..Z0..9
    int i = 0;
    if (isalpha(c)) i = c - 'A';
    else            i = c - '0' + 26;
    return i;
}

When you add a letter to the grid, strike it off this list by setting the position of that letter to a space. Then you can tell if a letter's been used before. And once the input key is finished you can scan this string for non-space chars and add those to the grid to finish it off.

Also, when you move on to the actual encoding part of the cipher, you might find that in addition to the 6x6 grid you might also store the row and col numbers for each letter in an array indexed by the letter (using get_index above) :

Code:

const int Size = 6;

struct {
    int row, col;
} table[Size*Size];

Having the row and col easily available for the input letters will help in the transposition. Note that the grid is used to look up the char for the resulting row and col (once they've been transposed as per the playfair scheme).

Unfortunately, professor's orders. No functions, no classes, etc. Just wants a simple copy'n'paste program to grade. Not exactly the best restriction if you ask me, but I have to deal with it regardless. But then again, I've seen worse restrictions.

And that still offers no solution to the toupper problem. "Shouldn't use the cast on the input to toupper." Should I input to a string, copy the string into an array of characters, and then toupper them, or what? Like I said, I've tried several different solutions, but they always seem to break my program on the second attempt after I stop and start it without touching the code.

You've read the input string into an array of chars so the use of toupper should just be:

Code:

 key[c] = toupper(key[c]);

That will work.
If you have other problems it may be due to the many other mistakes in your program, but you said you wanted to solve the logic flaws yourself.

Originally Posted by algorism

You've read the input string into an array of chars so the use of toupper should just be:

Code:

 key[c] = toupper(key[c]);

That will work.
If you have other problems it may be due to the many other mistakes in your program, but you said you wanted to solve the logic flaws yourself.

I think the flaw that caused the problem was in my convoluted cipher creator. Deleted all of that and it works fine, storing the array. I've also gone off of your suggestion of a "template array" that is refilled every time a new cipher is to be created, and the cipher is being created perfectly, even accounting for/removing repeated letters. Thanks for the help. Now for the encoding part...