Cipher text using a 2D array

Question

I have a simple program which ciphers text using a 2D array. You specify what letters to replace with what and it does it.

I understand the code is very repetitive and that I should probably use functions somehow, but I'm not sure how exactly to do that. Guidance (especially using code examples) would be much appreciated.

#include <conio.h> #include <stdio.h> // Main loop int main() { // 2D array - column 0 == original - column 1 == changed - init char array to 0 to get rid of garbage char repl_letter[26][2] = { {0}, {0} }; // User input to be ciphered char userinput[10]; // Ask user for key int replace_alpha; // Characters in array - user input int characters; int counter; // Count of characters - user input int count; // Alphabet repl_letter[0][0] = 'a'; repl_letter[1][0] = 'b'; repl_letter[2][0] = 'c'; repl_letter[3][0] = 'd'; repl_letter[4][0] = 'e'; repl_letter[5][0] = 'f'; repl_letter[6][0] = 'g'; repl_letter[7][0] = 'h'; repl_letter[8][0] = 'i'; repl_letter[9][0] = 'j'; repl_letter[10][0] = 'k'; repl_letter[11][0] = 'l'; repl_letter[12][0] = 'm'; repl_letter[13][0] = 'n'; repl_letter[14][0] = 'o'; repl_letter[15][0] = 'p'; repl_letter[16][0] = 'q'; repl_letter[17][0] = 'r'; repl_letter[18][0] = 's'; repl_letter[19][0] = 't'; repl_letter[20][0] = 'u'; repl_letter[21][0] = 'v'; repl_letter[22][0] = 'w'; repl_letter[23][0] = 'x'; repl_letter[24][0] = 'y'; repl_letter[25][0] = 'z'; // User input - string printf("What should I cipher?"); // User input characters = getchar(); // Count of characters count = 0; // Track array size and avoid buffer overflow while ((count < 10) && (characters != EOF)) { // Next character userinput[count] = characters; // Increment count of characters ++count; // Get another character characters = getchar(); } // Ask user for the key to cipher - loop through the alphabet - nth term for (replace_alpha = 0; replace_alpha < 26; replace_alpha++) { // Ask and use replace_alpha printf("Enter repl_letter for alph %d ", replace_alpha); // use %c for single character and put space before to omit newline - stored in column 1 of 2D array scanf(" %c", & repl_letter[replace_alpha][1]); } // REPLACING CHARACTERS for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[0]) { userinput[0] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[1]) { userinput[1] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[2]) { userinput[2] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[3]) { userinput[3] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[4]) { userinput[4] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[5]) { userinput[5] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[6]) { userinput[6] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[7]) { userinput[7] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[8]) { userinput[8] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[9]) { userinput[9] = repl_letter[counter][1]; } } for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[10]) { userinput[10] = repl_letter[counter][1]; } } printf(userinput); // Pause at the end of program for user convenience getch(); return 0; } 

Answer 1

First let's get rid of some of the repetition. You already seem to know hot to use loops so we just use a few more of them.

First one is the initialization of repl_letter. Ultimately you need an index from 0 to 25 and the corresponding letter of the alphabet from a to z. Now in ASCII the letters a to z come in successive order so we can make use of this:

for (int i = 0; i < 26; ++i) { repl_letter[i][0] = 'a' + i; } 

Second: You repeat this loop 10 times:

for ( counter = 0; counter < 26; counter++ ) { if (repl_letter[counter][0] == userinput[0]) { userinput[0] = repl_letter[counter][1]; } } 

and the only difference is the index into userinput. So you may as well have an outer loop for all the user input indices:

for (int user_input_index = 0; user_input_index < 10; ++user_input_index) { for (counter = 0; counter < 26; counter++) { if (repl_letter[counter][0] == userinput[user_input_index]) { userinput[user_input_index] = repl_letter[counter][1]; } } } 

A few additions:

1. You read the input character by character which isn't really that efficient. You could use getline although it's a bit more complicated to use.
2. You store the current user input character in a variable called characters - it would be better named current_character.
3. You could split the code into methods like initialize, read_input_stringread_replacements_characters, scramble_input. I leave that one for you to figure out :)

4. userinput is not NULL terminated and as such the printf instruction at the end may result in undefined behaviour due to the way strings work in C - essentially printf will simply go on and print characters until it has reached a '\0' which may cross the end of the buffer in your case since there is no guarantee that the last byte in the buffer is 0. The easy way to fix this is to make userinput larger by 1 byte (but still keep the maximum of 10 bytes for the input) and initialize the contents of it with '\0'. So this:

   char userinput[10]; 

   becomes this:

   char userinput[11] = { 0 };