PSET 2 Vigenere - argv[1] to string results in only single-digit key

Question

I had my Vigenere cipher up and running without any errors, but the ciphertext wasn't matching up like it was supposed to. I copied just the part of code meant to produce the key into a new program so I could see what it was producing:

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

int main(int argc, string argv[])
{
    //verify cmd line argument exists
    if(argv[1] == NULL)
    {
        printf("Remember to provide a key word after ./vigenere!\n");
        return 1;
    }

    //make key string from cmd line argument
    string key = argv[1];

    //verify key is only one word of only letters
    for(int i = 0, n = strlen(key); i < n; i++)
    {
        if(!isalpha(key[i]))
        {
            printf("The key must use only letters!\n");
            return 1;
        }

        if(argc > 2)
        {
            printf("The key can only be one word!\n");
            return 1;
        }
    }

    //finalize key by subtracting 65 from caps and 97 from lows
    for(int i = 0, n = strlen(key); i < n; i++)
    {
        if(isupper(key[i]))
            key[i] = key[i] - 65;

        if(islower(key[i]))
            key[i] = key[i] - 97;
    }

    for(int i = 0, n = strlen(key); i < n; i++)
    {
        printf("%i", key[i]);
    }

    printf("\n");
}

As I suspected, this is where my problem is -- the key is only one integer. For example, if I input "panda" as my command line argument, the key only results in 15, where it's supposed to result in {15, 0, 13, 3, 0}

What am I doing wrong? How do I make the key an array rather than a single number? I thought string key = argv[1]; would do that, but clearly not. Thanks in advance!

Answer 1

"How do I make the key an array rather than a single number?"

Actually, you already have, but just don't see it. ;-)

If you run your program as-is, except don't use an "a" or "A" in your key, I think you'll be surprised by the output. (You might want to add a space after %i in that next to last printf to make it easier to interpret the result. )

Go ahead, do that first and then come back! ;-)

Here's why you're not printing out more than one character. In testing, you are putting an 'a' as the second character in the key. An 'a' gets translated to 0. Here's where it gets interesting. After translating the entire key array, you are using strlen() to tell printf how much to print. The problem is that strlen() looks for 0x00 in a byte to find the end of the string. So, when it sees a 0 that was an "a", it thinks it's the end of the string marker \0, which is 0x00! Oops!

There's a better way. You should calculate the length of the key one time, just after you validate the key. Set int n = strlen(key); once, and then use n over and over as you have in your code. (You'll need it later anyways.) Why repeat the same calculation for the same result? By not recalculating it each time, you've eliminated several duplicate calculations, since the key length doesn't change, so the program is a bit more efficient, and the strlen/0 problem is eliminated.

Except for not understanding how strlen() works, you're probably right on track! Happy programming!

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