I'm really stuck on vigenere. I've gotten close, but I think there's an issue with the way I've set up the Key and how it wraps around. Here's my program:

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

int main(int argc, string argv[])
if (argc != 2)
    printf("Bad command or file name \n");
    return 1;
}//quits if there are not exactly 2 command line arguments

string key = (argv[1]);
int KeyLen = strlen(key);

for (int i = 0; i < KeyLen; i++)
    if (!isalpha(key[i]))
        printf("Bad command or file name \n");
        return 1;
    }//quits program if any letter in the key is not alpha


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

for (int i = 0, j = 0, ptlen = strlen(plain); j < ptlen; j++)//iterate over plaintext
    key[i] = toupper(key[i]);//makes all letters in key uppercase
    int cipherKey = (((key[i]) - 'A') % KeyLen);//get a 0-based int index for the key

    if(isalpha(plain[j]) && isupper(plain[j]))
        printf("%c", ((((plain[j] - 'A') + cipherKey) % 26) + 'A')); //print capital encrypted letter
        i++;//advance one letter in key
    else if(isalpha(plain[j]) && islower(plain[j]))
        printf("%c", ((((plain[j] - 'a') + cipherKey) % 26) + 'a')); //print lowercase encrypted letter
        printf("%c", plain[j]);
return 0;

Specifically I think the issue has to do with lines 32-35.

key[i] = toupper(key[i]);

int cipherKey = (((key[i]) - 'A') % KeyLen);

I'm not quite sure if the 'toupper' function is correctly used, or maybe my % value is off somehow?

The cipher works but only if it doesn't need to wrap around. I've been at this for countless hours and days on end. Any advice or help would be greatly appreciated!


Take a close look at this line and think very carefully about what the modulo operation is being applied to:

int cipherKey = (((key[i]) - 'A') % KeyLen);

It coverts a key letter to a number between 0 and 25, and then tries to wrap around based on key length, by applying the % operation to the value of the key, not the position.

This has two bad results. First, it corrupts the value of any key letter who's value after conversion is larger than the length of the key (something you wouldn't see if testing with letters at the beginning of the alphabet). Second, i is free to increment without bound beyond the end of the key.

The wraparound % should be applied to the index of the key, i, and not to the value of the letter. I'll let you mull over how to do that for a bit. ;-)

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

  • Thanks for your help Cliff! I understand the concept much more thoroughly now. I'm still trying to reason out how to get an accessible 'index position' of the key (i) that doesn't rely on the actual value of 'i'. – Alex Nov 22 '17 at 18:15
  • It does rely on i. Here's a hint. key[i + 3] Don't take it literally, it's just an example of modifying i. ;-) – Cliff B Nov 22 '17 at 18:21
  • Wow - I spent SO much time and got so into the weeds here, and then I went back and thought about your comment and one simple fix has my program working according to spec! Thanks Cliff! – Alex Nov 23 '17 at 17:40

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .