PSET2 Vigenere issues

Question

I've been stuck on Vigenere for a while now. I believe my code shifts the keyword correctly, but I am having trouble shifting the plaintext by that keyword. I feel like I'm missing something major in the code for the wrap around.

Any help is appreciated.

Thanks

int main(int argc, string argv[])
{

    if (argc == 2)
    {
        string k = argv[1];

        for (int i = 0 , m = strlen(k); i < m; i++)
        {
            if (isalpha(k[i]) && isupper(k[i]))
            {
                k[i] = (k[i] - 65);
            }
            else if (isalpha(k[i]) && islower(k[i]))
            {
                k[i] = (k[i] - 97);
            }
            else
            {
                return 1;
            }
        }

        string p = get_string("plaintext: ");
        string c = p;

        int m = strlen(k);
        int j = 0;

        for (int i = 0; i < strlen(p); i++)
        {


            if (isupper(p[i]) && isalpha(p[i]))
            {
                c[i] = p[i] - 65;
                c[i] = (p[i] + k[j % m]) % 26;
                c[i] = c[i] + 65;
                j++;
            }
            else if ( islower(p[i]) && isalpha(p[i]))
            {
                c[i] = p[i] - 97;
                c[i] = (p[i] + k[j % m]) % 26;
                c[i] = c[i] + 97;
                j++;
            }

        }
        printf("ciphertext: %s\n", c);
    }
    else
    {
        printf("error!\n");
        return 1;
    }
}

Answer 1

Did you know that strlen() is based on the location of the end of string marker, \0? The EOS marker is actually represented as binary 00000000 or 0x00. You might ask why this is important. Well, it means that if there's a 'a' or 'A' in the key, it's going to break the code.

In the first for loop, the key is converted from letters to numbers from 0 to 25 inclusive. That means the a's are converted to 0, which happens to represent the EOS character. So, if there's an 'a' in the key, it will become an end of string marker and the code will wrap back to the beginning of the key.

The technique is good, but you just need to get the string length and store it in a var before the for loop.

Now, some programming tips.

isupper() and islower() both imply isalpha(), so there's no need to use isalpha when you use them.

About code structure: It is a common action for new programmers to structure a program like this:

int main(void)
{
    if(argc = <a number>)
    {
        // vast bulk of program
    }
    else
    {
        //error message
    }
 }

While this is technically valid, it's considered a bad practice. The reason is that it might be necessary in the future to modify the code. Another programmer, or even the original author many months or years later, may not recognize that the error message at the very end of the program is linked all the way back to the test at the very beginning. Imagine this in a program with 10k lines of code and searching for the closing curly brace for the 'if' block of code!

The best practice is to compartmentalize code by testing for the failure at the start so that it is completely independent of code that follows. There is no need for an else clause because the code will either exit the program in error or will continue to execute. In this case, it would look something like this:

int main(void)
{
    if(argc != <a number>)
    {
        //error message
    }

    // vast bulk of program

 }

If the program ever had to be modified or updated, it's easy to figure out the structure and there would be no else clause at the far end of the program!

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