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.

I cannot reproduce this for your input `aaaa` and `bbbbbbb`, although I am not using any `string` type, just assigning `char *k = argv[1];` and `char *code = argv[2];` the rest is as you posted. I suggest you print out the value of `int klen = strlen(k);` — Weather Vane, Oct 05 '16 at 15:37
I think that your third printf should be `printf("%c", (((code[a] - 'a') + (k[b%klen] - 'A'))%26) + 'a');`. — Bob__, Oct 05 '16 at 15:43
Bob, thanks for that. That was definitely part of the issue. I have updated my code, but now instead of `bbbbbNN` I'm getting `bbbbbhh` I still seem to be having trouble with wrapping the key around the code — Michael McGuire, Oct 05 '16 at 15:48
The example does not have any uppercase letters for the key, so I did not get a wrong output. But I still ask: what is the value of `klen` for the key `aaaa`? — Weather Vane, Oct 05 '16 at 15:52
I can't reproduce your issue. [Here](https://ideone.com/r8DYrg), with the same logic but different input method. — Bob__, Oct 05 '16 at 16:18
Over here the key index does wrap. With inputs `abcd` and `bbbbbbbbb` I get the output `bcdebcdeb` and as I said too, different input method. — Weather Vane, Oct 05 '16 at 16:18
From both of your responses it seems that my `for loop` is fine, and the problem lies somewhere with my string, seeing as how you've both used `char` instead — Michael McGuire, Oct 05 '16 at 16:25

score 0 · Accepted Answer · answered Oct 05 '16 at 16:45

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;
}

Thanks for the response. I'm still very new to this and don't understand some of what you wrote, but I'll try to get the key set before the code and see what happens. — Michael McGuire, Oct 05 '16 at 17:17
I've taken your advice and processed they key outside of the code loop and it works! It's still a little messy and can be simplified but at least it works now, and I can work on improving the efficiency of it. Thanks again. — Michael McGuire, Oct 05 '16 at 19:46

Vigenere Cipher. Code output

1 Answers1