vigenere.c cs50 plaintext for loop starts after the key array loop ends

Question

i need help in the sense that if i run my program with the key being one letter, whether capital or lowercase, it works properly, but if more than one letters in the key, the last letter is used for all the letters in the plaintext. why? because, using printf, i found out that the loop for finding if the jth char of key is letter or not, then i tell the var key_val that it == equals key[j] but once the for loop for that is done, instead of going to the next for loop (plaintext) and coming back for this loops second stage, it completly finishes this key loop (rendering it to the last letter as key_val) and than the plaintext value gets to use the last char for everything.

my code...

    #include<stdio.h>
    #include<cs50.h>
    #include<stdlib.h>
    #include<string.h>
    #include<ctype.h>
    int key_val;
    int main(int argc, string argv[])
    {
        if (argc != 2) {
            printf("type program name and key! no more and no less!");
            return 1;
        }
        string key = argv[1];
        string plaintext = GetString();
        for(int j = 0,n = strlen(key);j < n; j++ )
        {
            key_val = key[j];
            if (isalpha((unsigned char)key[j])){
                if (islower((unsigned char)key[j]))
                {
                key[j] = key[j] - 104;
                }
                else if (isupper((unsigned char)key[j]))
                {
                key[j] = key[j] - 72;
                }
            }
            else if (!isalpha((unsigned char)key[j])){
                printf("Keyword must only contain letters A-Z and a-z\n");
                return 1;
            }


}

    for(int i = 0,n1 = strlen(plaintext);i < n1; i++ )
{
    int upalpha = plaintext[i] - 'A';
    int lowalpha = plaintext[i] - 'a';
    int resultupper = ((upalpha + key_val + 7) % 26 + 'A');
    int resultlower = ((lowalpha + key_val + 7) % 26 + 'a');
    if (isalpha((unsigned char)plaintext[i])){
        if (islower((unsigned char)plaintext[i]))
        {
            printf("%c", resultlower);
        }
        if (isupper((unsigned char)plaintext[i]))
        {
            printf("%c", resultupper);
        }
    }
    else {
        printf("%c", plaintext[i]);
    }
}
printf("\n");
    }

Answer 1

First of all, you should get used to doing a better job of formatting your code so that you can see which opening and closing curly braces go together. style50 is your friend!

From your explanation, it appears that you believe that the first for loop will execute one pass, then the second loop will execute one pass, and then the code will return to the first loop to execute the next pass. It doesn't work that way.

The first loop will execute repeatedly, until the test condition of that loop fails. THEN, the code that follows it will execute. In this case, the code that follows is the second for loop. It will execute all of its passes at that time. And just in case you were thinking it, no, these are not nested for loops. (BTW, if you're thinking about it, nested for loops don't work to solve this problem.)

So, the first loop is processing the key string and assigning the last letter to key_val on the last pass, which will then be used by the second for loop. The code that follows really serves no purpose because key_val has already been assigned. (This would be true, even if it were cycling through the key during encryption.)

While the first loop would be good to go through the key string to validate it, it won't help you with the encoding. During encoding, the index for the key string has to be incremented ONLY when a letter is encoded and NOT when a non-alpha like a number, space or punctuation, etc. is processed.

Next, I don't understand why you are subtracting 72 and 104 from the ASCII values from the key values. Those values make no apparent sense to me. Nor does the need to add 7 back in later.

More tips:

key_val is declared as a global variable outside of main(). There is absolutely no need for a global var in this program. Globals should only be created when absolutely needed. If you can't articulate a good reason for one, don't create one. Globals introduce the opportunity for big problems, particularly when there are multiple functions and multiple programmers writing different parts of the code.

Review the man pages for the isalpha(), islower(), isupper(), tolower() and toupper() functions. For example, the tolower() function will return the lower case version of an upper case letter, or it will return the same character that was given as the parameter (a number, a lowercase letter, punctuation, etc.) There's no need to test whether it is an uppercase letter. Just call it and it will do the job.

You still have a bit of work to do, but this should get you going. If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)