vigenere output wrong, but believe it's a coding problem, not a logic problem

Question

Thanks in advance to anyone who can point out what I've done wrong. I can complete the exercise on paper correctly. I believe the problem is I don't know how to write the code to make it do what I want.

I had no trouble with Caesar and expected Vigenere's to be a simple variation, i.e. all it required in addition was to apply a key which changed to the plaintext.

I'm to the point where I'm randomly trying modifications to the code just to see what happens and if I can find the flaw via analyzing the output.

I believe the first part of my code is ok up until the point where I want to modulo on j, i.e. the index of the keyword, for the strlen(keyword).

int main(int argc, string argv[])
{
// checks for single command line argument and keyword all letters; quits if false
if (argc != 2)
{
    printf("Usage ./vigenere keyword");
    return 1;
}

string keyword = argv[1];

for (int j = 0, n = strlen(keyword); j < n; j++)
    // verifies keyword all alpha chars; quits if false
    if (isalpha (keyword[j]))
    {

        // checks if keyword char is uppercase; converts to alpha index
        if (isupper(keyword[j]))
        {
            keyword[j] = keyword[j] - 'A';
        }

        // checks if keyword char is lowercase; converts to alpha index
        if (islower(keyword[j]))
        {
            keyword[j] = keyword[j] - 'a';
        }

    }

    else
    {
        printf("Keyword may only use alphabetic characters");
        return 1;
    }

All I wanted thereafter was to loop thru the plaintext adding the keyword[j] value to it. I expected to determine the value of keywore[j] by modulo-ing on j for the strlen(keyword):

        for (int i = 0, m = strlen(plaintext); i < m; i++)
    {   
        // checks if plaintext char is a letter
        if (isalpha (plaintext[i]))
        {

            for (int j = 0, n = (keyword[j % strlen(keyword)]); j < n; j++)
            {

                // checks if uppercase, makes alpha index, applies key, makes ASCII
                if (isupper(plaintext[i]))
                {
                    char ciphertext = (((plaintext[i] - 'A') + keyword[j]) % 26) + 'A';
                    printf("%c", ciphertext);
                }

                // converts lowercase chars to alpha index, applies key, makes ASCII
                if (islower(plaintext[i]))
                {
                    char ciphertext = (((plaintext[i] - 'a') + keyword[j]) % 26) + 'a';
                    printf("%c", ciphertext);
                }

            }
        }
            // prints non-alphabetical chars unchanged   
        else
        {
            printf("%c", (plaintext[i]));
        }

    }

}

printf("\n");

return 0;
}

I actually get what appears to be a closer result by pulling the modulo out--closer being defined as the right number of chars, spaces being observed, and case being preserved--which logically I don't understand at all.

for (int i = 0, m = strlen(plaintext); i < m; i++)

I'm really sorry. I've read plenty of other posts. I just can't see it and I've been staring at it for hours. Up until now, whenever I've been able to work it out on paper, I've been able to get past Check 50. A million thanks!

Answer 1

It's a logic and a coding problem. (It's not a syntax problem though.) You're on the right track in that you know you need to track the indexes of both the key and the plain text separately. You just picked the wrong way to do it.

While it seems like a loop within a loop should work, that's simply not the case. By doing that, here's what's happening. You select a letter from the plain text to encode with the outer loop. Next, you start running the inner loop. That inner loop is going to operate on that single letter with each letter of the key string! Then, it's going to repeat the process with the next letter to be encoded.

Instead of using the inner for loop, you should be stepping through the key by setting j=0 before the outer loop and simply incrementing j each time a letter is encoded. Of course, you do need to check j against the length of the key and reset it when you've consumed all of the elements of the key.

If this answers your question, please click the check mark to accept this and remove the question from the unanswered pool. Let's keep up on forum maintenance. ;-)