Vigenere Cipher. Code output

Question

I've been working on cs50 pset2, and I thought I had the vigenere cipher down after working on it for a few days. This code is meant to take an alphabetical argument(argv[]) given by the user, and use that as a key to crypt a phrase given by the user(string) by its number in the alphabetical index. For example, if you give the argument 'abc' and the string 'cat' then the output should be 'cbv'(a moving 0, b moving 1, c moving 2) The argument should also wrap around so that if the string is longer, the argument will wrap to its first character and continue until the string has ended.

This is what I have for code:

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


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


    if(argc != 2)
        {
            printf("Try again\n");
            return 1;
        }
    string k = (argv[1]);

    int klen = strlen(k);


    for(int x = 0; x < klen; x++)
        {
            if(isalpha(k[x]))
                {
                    if(isupper(k[x]))
                        {
                            k[x] = tolower(k[x]);
                        }

                        k[x] -= 'a';
                }
            else
                {
                    printf("Try again\n");
                    return 1;
                }
        }

    string code = GetString();

    int clen = strlen(code);

    for(int a = 0, b = 0; a < clen; a++)
        {
            if(isalpha(code[a]))
                {
                    int key = k[b%klen];
                    if(isupper(code[a]))
                        {
                            printf("%c", (((code[a] - 'A') + key)%26) + 'A');
                            b++;
                        }
                    else
                        {
                            printf("%c", (((code[a] - 'a') + key)%26) + 'a');
                            b++;
                        }
                }
            else
                {
                    printf("%c", code[a]);
                }
        }
    printf("\n");
}

The code seems to work for the length of the key +1. For example, I input an argument of 'aaaa'

Then input a string of 'bbbbb' and receive 'bbbbb' correctly.

However, if I input the same 'aaaa'

Then input a string longer than the key +1 'bbbbbbb' I receive 'bbbbbNN'

I believe I have an issue with my order of operations but have tried moving parenthesis around to no avail. I was hoping someone could point me in the right direction as to why my key isn't wrapping properly.

Answer 1

Your biggest risk with code like this is all the similar, repetitive clauses. A bug in just one is hard to track done. And doing any processing on the key, while processing the code, is just inefficient.

Here's a rework that completely processes the key before processing the code and tries to get the processing down to just one case. See if it works any better for you:

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

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

    if (argc != 2)
    {
        fprintf(stderr, "Try again\n");
        return EXIT_FAILURE;
    }

    string key = strdup(argv[1]);

    size_t key_length = strlen(key);

    for (int x = 0; x < key_length; x++)
    {
        if (isalpha(key[x]))
        {
            if (isupper(key[x]))
            {
                key[x] = tolower(key[x]);
            }

            key[x] -= 'a';
        }
        else
        {
            fprintf(stderr, "Try again\n");
            return EXIT_FAILURE;
        }
    }

    string code = GetString();
    int code_length = strlen(code);

    for (int a = 0, b = 0; a < code_length; a++)
    {
        if (isalpha(code[a]))
        {
            int start = isupper(code[a]) ? 'A' : 'a';

            printf("%c", (((code[a] - start) + key[b++ % key_length]) % 26) + start);
        }
        else
        {
            printf("%c", code[a]);
        }
    }

    printf("\n");

    free(key);

    return EXIT_SUCCESS;
}