How do I fix the looping from 'z' back to 'a' in my Vigenere Cipher

Question

My Vigenere cipher has problems. When I input my message, the result comes out fine, but if the letter goes past 'z', it doesn't loop back to 'a', and is printing out other ascii characters. Moreover, when I put the message in, I sometimes get more characters than I needed. ex: key is hello, message is mmmmm(I know, not much of a message, but it's an example), and the output is tqxx{{. PLEASE HELP!!!!

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

int main( int argc, char *argv[])
{
    char *k;
    if (argc!=2)
    {
        printf("Give one argument, not 2, not 3, not 4 , and not any other amount other than one");
        printf("\n");
        return 1;
    }
    k = argv[1];
    printf("What is the message? ");
    string message = get_string();
    if (strlen(k)<strlen(message))
    {
        printf("Invalid response\n");
        return 0;
    }
    for (int i = 0, l = 0; i < strlen(message); i++, l++)
    {
        int x;
        if (message[i]>='A' && message[i] <='Z')
        {
            message[i] = message[i]-'A';
            message[i] = message[i] + (k[l] - 'A') % 26;
            message[i] = message[i] + 'A';
            message[i] = (char) x;
            if (x > 90)
            {
                x = x - 26;
                message[i] = (char) x;
            }
            printf("%c", message[i]);
        }
        if(message[i] >='a' && message[i] <= 'z')
        {
            message[i]= message[i]-'a';
            message[i] = message[i] + (k[l] - 'a') % 26;
            message[i] = message[i] + 'a';
            printf("%c", message[i]);
        }
      if ((message[i] < 'A') || (message[i] > 'z') || (message[i] > 'Z' && message[i] < 'a'))
        {
            printf("%c", message[i]);
        }
    }
    printf("\n");
    return 0;
}

Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a Minimal, Complete, and Verifiable example. — too honest for this site, Jul 03 '17 at 16:16
I think it is easiest if you convert the current letter to an index into the alphabet (`a` as 0, `z` as 25; similarly for A..Z), convert the key letter to an index into the alphabet, add the two indexes modulo the size of the alphabet (giving a result that is an index into the alphabet in the range 0..25), and convert the index back to the appropriate letter (add the index to `'a'` or `'A'`). Since you'll process the key many times in general, you could consider preprocessing it into the index values so you don't do that per character. Also consider writing functions — tiny inline functions. — Jonathan Leffler, Jul 03 '17 at 18:14
I bet you'd find answers to your problems in any of the 10 'related' questions listed on the RHS of the page. There's even one that mentions 'extra characters' in the title. — Jonathan Leffler, Jul 03 '17 at 20:20

score 0 · Answer 1 · answered Jul 03 '17 at 16:28

You should put brackets around the message[i] + (k[l] - 'a') part. The modulo goes only for the second part - (k[l] - 'a') - while you want it to apply to the whole expression.

Also, you assume k[l] is the same case as message[i] (upper/lowercase), but that's not necessarily true. You should have another condition in each of the inner branches, or (preferable), calculate the numeric values of k's letters and store them as numbers in range 0-25.

Third, why is x there? It's used when uninitialized, and generally I don't see a reason for it's existence.

Last thing, you should probably set l to zero again when it reaches the end of k, in case the message is longer than the key.

How do I fix the looping from 'z' back to 'a' in my Vigenere Cipher

1 Answers1