How to reuse(loop) key in vigenere cipherkey cs50

Question

I was making a program for Vigenere cipher. I made the program print the cipher text successfully. But, I can't loop the key. so if my key was 'abc' and my plain text was hello, it should print 'hfnlp' not 'hfn'.

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

int main(int argc, string argv[])
{
    if(argc != 2)
    {
        printf("\aError\n");
        return 1;
    }
    else
    {
        string a = argv[1]; // converts argv[1]

        printf("plaintext: ");
        string b = get_string(); // takes the plaintext

        printf("ciphertext: ");

        for(int c = 0, d = strlen(a); c < d; c++)
        {
            for(int e = 0, f = strlen(b); e < f; e++)
            {
                if(islower(a[c]))
                    {
                        printf("%c\n",  b[e] + ( (a[c] - 97) % 26) ); // works for lowercase letters
                        return 0;
                    }
                    else if(isupper(a[i]))
                    {
                        printf("%c\n",  b[e] + ( (a[c] - 65) % 26) ); // works for uppercase letter
                    }
                    else
                    {
                         printf("%c", b[e]); // works for non alphabetical inputs
                    }

                    if(true)
                        break;
            }
   }

    printf("\n");
}
}

Answer 1

Your choice of single-letter variable names is odd; it makes it harder to work with your code. I'm not a fan of long names either, but intermediate length variable names (2-8 characters — except for some stylized single-letter names (c, i, j, k, p, s) — is typically appropriate).

You've got trouble because if your key is 6 characters and your string is 24 alphabetic characters, you'll attempt output 144 alphabetic characters because of the loop structure. You only need a single loop that iterates over the characters in the plain text. You have a separate variable that cycles over the length of the key, resetting back to the start when it runs out. In this code, the key length is in keylen (you used d) and the offset (index) into the key is in keyoff (you used c) — but the key is still in a because that's what you used. Left to my own devices, I'd probably use text (or maybe plain) in place of b, textlen in place of f, and I'd use i instead of e for the loop variable. If I wanted to use short indexes, I might use k instead of keyoff. I might also edit the string in situ and print the whole string at the end.

This code also ensures that the alpha characters in the key are in lower case. It doesn't ensure that the key is all alpha; it arguably should and it would be trivial to do so since the key is scanned anyway. As it stands, it is a case of GIGO — garbage in, garbage out.

The code converts the input letter (a-z or A-Z) into an 'offset into the alphabet' by subtracting a or A, converts the key letter into an offset into the alphabet, adds the two offsets modulo 26 (number of letters in the alphabet), and converts the offset back into a letter of the appropriate case.

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

int main(int argc, string argv[])
{
    if (argc != 2 || strlen(argv[1]) == 0)
    {
        fprintf(stderr, "Usage: %s key < text\n", argv[0]);
        return 1;
    }

    string a = argv[1];
    int keylen = strlen(a);
    for (int i = 0; i < keylen; i++)
        a[i] = tolower((unsigned char)a[i]);
    printf("key: %s\n", a);

    printf("plaintext: ");
    string b = get_string();

    printf("ciphertext: ");

    int keyoff = 0;
    // Step through each character of the plain text.  Encrypt each
    // alpha character with the (lower-case) key letter at a[keyoff],
    // incrementing keyoff.  Don't increment key offset when processing
    // non-alpha data.
    for (int e = 0, f = strlen(b); e < f; e++)
    {
        if (islower(b[e]))
            printf("%c", ((b[e] - 'a') + (a[keyoff++] - 'a')) % 26 + 'a');
        else if (isupper(b[e]))
            printf("%c", ((b[e] - 'A') + (a[keyoff++] - 'a')) % 26 + 'A');
        else
            printf("%c", b[e]);
        if (keyoff >= keylen)
            keyoff = 0;
    }
    printf("\n");
    return 0;
}

When compiled to the program vc41 and run, it produces, for example:

$ vc41 abcdef
key: abcdef
plaintext: The quick brown Fox jumped over the lazy Dog.
ciphertext: Tig tyncl dusbn Gqa nzmqgg saes vki qaaa Gsl.
$

I generated an 8-letter random key (it was GZlfmTMk) and ran the code on a number of 'complete alphabet' strings:

$ vc41 GZlfmTMk
key: gzlfmtmk
plaintext: Pack my box with five dozen liquor jugs.
ciphertext: Vznp yr nyd vtyt yufk czeqg xswtzw vnsc.
$ vc41 GZlfmTMk
key: gzlfmtmk
plaintext: The five boxing wizards jump quickly.
ciphertext: Zgp kuoq luwtss pujgqox vnyz wtthwek.
$ vc41 GZlfmTMk
key: gzlfmtmk
plaintext: How vexingly quick daft zebras jump.
ciphertext: Nnh aqquxmkj vgbou jzqy lxnbgr uzyi.
$ vc41 GZlfmTMk
key: gzlfmtmk
plaintext: Bright vixens jump; dozy fowl quack.
ciphertext: Hqtltm hsddyx vnyz; jnkd rhiv wtlhw.
$ vc41 GZlfmTMk
key: gzlfmtmk
plaintext: The quick brown fox jumps over the lazy dog.
ciphertext: Zgp vgbou hqzbz yah ptxue hhox ssj xtli jnr.
$

(I'll note in passing that on a Mac running macOS Sierra 10.12.6 using GCC 7.1.0, this code links without including the (new) CS50 library — there is a system function get_string() that has a different interface to the CS50 version that satisfies the reference but crashes the program. However, it isn't documented by man get_string, so I'm not sure what the system function of that name actually does; I haven't chased it more actively, or found out how extensive the problem is. That caused me a headache that the old CS50 library didn't. Grumble…)

Answer 2

fix like this

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

int main(int argc, string argv[]){
    if(argc != 2 || !*argv[1]){
        printf("\aError:The number of command arguments is incorrect.\n");
        printf("Usage: %s key_string\n", argv[0]);
        return 1;
    }
    //Since it is `return 1;` in the if-statement, 
    //the else clause is unnecessary (unnecessarily deepening the nest)
    string key = argv[1];//It does not convert.
    size_t i, key_len;
    unsigned char curr_char;
    for(i = 0; (curr_char = key[i]) != '\0'; ++i){
        if(!isalpha(curr_char)){
            printf("\aError:Only the alphabet can be specified as the key.\n");
            return 1;
        }
        key[i] -= islower(curr_char) ? 'a' : 'A';//Convert to Deviation
    }
    key_len = i;
    i = 0;

    printf("plaintext : ");
    string plain = get_string();

    printf("ciphertext: ");

    for(size_t j = 0; (curr_char = plain[j]) != '\0'; ++j){//Scan of plain text should be the main loop.
        if(isalpha(curr_char)){
            char base_char = islower(curr_char) ? 'a' : 'A';//Avoid using magic numbers

            putchar(base_char + (curr_char - base_char + key[i]) % 26);//Make the same process one
            if(++i == key_len)
                i = 0;//reset key index
        } else {
            putchar(curr_char);//non alphabetical inputs
        }
    }
    printf("\n");
    free(plain);
}