7
\$\begingroup\$

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;
}
\$\endgroup\$
4
  • 2
    \$\begingroup\$ You should not modify your code after receiving an answer, and in particular not remove it from the question. Have a look at what you may and may not do after receiving answers for more information. \$\endgroup\$
    – Martin R
    Jul 31, 2016 at 9:07
  • \$\begingroup\$ @MartinR Is it possible to host the code somewhere else ? Code will be always available. \$\endgroup\$ Jul 31, 2016 at 9:09
  • 1
    \$\begingroup\$ @TheodoreFalcone No. See this FAQ. \$\endgroup\$
    – Mast
    Jul 31, 2016 at 9:12
  • \$\begingroup\$ No problem. Getting used to the rules. Newbie here. \$\endgroup\$ Jul 31, 2016 at 9:20

1 Answer 1

5
\$\begingroup\$

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_string read_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 };
    
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.