Wrong output with Vigenere

Question

I've been actually checking the code for hours without figuring out what mistake it's got. The answers are basically all wrong except for the first letter, which is always right. Sometimes other letters are right as well. I can't find any sort of pattern the mistakes follow. Please help me find out what's wrong with the program.

#include <stdio.h>
#include <cs50.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, string argv[])
{
    string key=argv[1];
    for(int i=0;i<strlen(key);i++)
    {
        if (key[i]>=65 && key[i]<=90)
        {
            key[i]=key[i]-65;
        }
        else if (key[i]>=97 && key[i]<=122)
        {
            key[i]=key[i]-97;
        }
    }
    string plaintext = GetString();
    for(int n=0;n<strlen(plaintext);n++)
    {
        int q=n;
        if (q>strlen(key)-1)
        {
            while(q>strlen(key)-1)
            {
                q=q-strlen(key);
            }
        }
        if ((plaintext[n]>=65&&plaintext[n]<=90)||(plaintext[n]>=97&&plaintext[n]<=122))
    {
        if (plaintext[n]>=65 && plaintext[n]<=90 && plaintext[n]+key[q]>90)
        {
            while (plaintext[n]+key[q]>90)
            {
            plaintext[n]=plaintext[n]-26;
            }
        }
        else if (plaintext[n]>=97 && plaintext[n]<=122 && plaintext[n]+key[q]>122)
        {
            while (plaintext[n]+key[q]>122)
            {
                plaintext[n]=plaintext[n]-26;
            }
        }
        plaintext[n]=plaintext[n]+key[q];
     }
     printf("%c",plaintext[n]);
}
printf("\n");
}

Answer 1

There are a couple of problems to fix. The first one is very subtle. In your first FOR loop, you convert each letter of the key to a number from 0 to 26. (A good idea that eliminates the need for doing it repeatedly later and eliminates the need to check the case of the key characters later.) However, because of the way you did it, it breaks after the first pass.

The FOR loop is dependent on the string length: i<strlen(key); On the first pass, the first letter is changed from a valid ASCII letter value to a number between 0 and 25. When you come back up to the start of the loop, this test is re-evaluated and strlen() sees something different. If the first letter in the key happens to be a or A, then it sees 0. That happens to be the end of string value, so strlen(key) = 0. Oops. The rest of the key remains unchanged. It's possible that there could be other adverse effects generated by other letters.

You can get around this by storing strlen(key) in a separate variable at the top of the program and using that var instead of recalculating strlen() each time it is needed.

Next, you have tied q, the index for selecting the correct key element, to n, the index for the plaintext. These two indexes are independent and should not be linked. n will change with every character processed in plaintext, while q will only increment when an actual letter is encoded. Spaces, numbers, punctuation or any non-alpha character will not cause q to increment. Also, it has to reset to 0 when it gets to strlen(key).

It looks like there are more issues as well, but I don't want to take all of your fun away from you. ;-) Here are some other hints that might help. Take a look at the following functions: isalpha(), isupper(), islower(). Also be very aware that a char behaves like a signed 1 byte integer. That means that if the number goes above 127, it is treated as a negative number represented in twos complement form(google it). What may seem like 192 really isn't and won't test true as > 127. There are other questions on this forum that go into this.

If this answers your question, please click on the check to accept. Let's keep up on forum housekeeping. ;-)