Cipher text using a 2D array

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
#include

// 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 = ge

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:

• 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.

• You store the current user input character in a variable called characters - it would be better named current_character.

• You could split the code into methods like initialize, read_input_string read_replacements_characters, scramble_input. I leave that one for you to figure out :)

-
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 };

StackExchange Code Review Q#136439, answer score: 5