I have been trying for way too long to make my Vigenere cipher work, but now I am so confused I don't even know what I am doing. I need some guidance in what it is that I am doing wrong. When I ran check50 the argc part of the code where a single-command argument works, but the ciphering itself just doesn't work. I really appreciate the help!

Assuming i is the index of the current character in the plaintext, 65 is the ASCII value of the uppercase 'A', word is the key used to cipher the plaintext and j % strlen(word) is the index of the current char in that key, what's wrong with this formula?

plaintext[i] - 65 + word[j % strlen(word)]) %26 + 65;
  • 2
    The Honor Code says not to post your code in public. Please consider editing it out and explaining your problem in plain English. What exactly happens? What tests have you run? What exactly does "not working" mean? Does it only cipher some letters? No letters? Too many? Sometimes by explaining the problem, you will see a new way to actually code it.
    – curiouskiwi
    Commented Jun 11, 2014 at 8:53
  • Flag the post and choose the last option when it's the case.
    – ItsEric
    Commented Jun 11, 2014 at 12:20
  • It is okay to post "wrong" code that does not work. You cannot post working code. The following is listed under Reasonable in our project description: "Sending or showing code that you’ve written to someone, possibly a classmate, so that he or she might help you identify and fix a bug."
    – Azurespot
    Commented Jun 16, 2014 at 5:56
  • Sending or showing code to 'someone' does not include posting it in public for the entire internet to see, though. Unless we are following different rules on stackexchange than we are on our sub-reddit. I think someone from staff really needs to clarify.
    – curiouskiwi
    Commented Jun 18, 2014 at 0:25

4 Answers 4


Youre mingling things up a little bit, so I suggest you take some fresh air and let your code be more open, for example, change the way you try to print and cipher everything at the same time, use a for loop to cipher each and every letter seperately one by one, that'll allow you to see where exactly your'e going wrong, as itll make things more open.

But, as for the main problem, its the ciphering, I suggest that to cipher each letter do this:


ciphered[i] = 'A' + (plaintext[i] - 'A' +cipherer) % 26;

where cipherer is:

cipherer = (k[ki % klen] - 'A') % 26;

Un-Capital(small letters)

ciphered[i] = 'a' + (plaintext[i] - 'a' +cipherer) % 26;

where cipherer is:

cipherer = (k[ki % klen] - 'a') % 26;

I have chosen not to give you the whole vigenere.c file code so that the pset still remains a bit of a challenge for you.


  • Thanks, I think I have a better idea of how to make it work!
    – emece
    Commented Jun 11, 2014 at 15:09
  • Thanks for accepting the answer, @emece !
    – Nib
    Commented Jun 13, 2014 at 14:39
  • @Nib would you have to cast plaintext[i] into an integer in order to use it, or is that not necessary? Commented Jul 3, 2015 at 21:20

I think instead of making things complex by doing all calculations inside printf statement do it outside to make yourself clear what's going wrong. Here you wont be able to make it up that is the plaintext[i]-(??) making trouble or word[j%(????)] making trouble or something else.

I suggest instead of directly converting the plain text to cipher text in printf generate key from the arguments argv[1] and store it in an array of same length as your string from user.

Then use same process as in Caesar Cipher for each character for encryption since now u have key for each character stored in an array , so it would be easy now.

And don't forget that in the key you generate A and a stands for 0 while Z and z stands for 25 and spaces inside input text has to be reflected in the key also, so generate your key accordingly.

Hope this would help.

  • Thanks for your input!
    – emece
    Commented Jun 11, 2014 at 15:01

First, a good design tip is to declare a variable to hold the length of word to avoid calling strlen() every iteration of the loop. This can be done this way:

for (int i = 0, n = strlen(word); i < n; i++)
    // do something

Second, nothing will yell at you if you just used 'A', 'a' or any other char in a mathematical expression since they're actually just numbers underneath the hood. This may even seem more readable.

Lastly, the remainder operator (i.e., %) has a higher precedence than the addition operator (i.e., +). This simply means, given the expression:

x + y % z

the expression y % z is evaluated first, in this case, then x is added to the result.

To solve this issue you'll need to use parentheses like that:

(x + y) % z

This way, you'll get the expression x + y evaluated first, then you'll take the remainder of dividing the result by z.

  • 'A' has ASCII value of 65 and not 67. ASCII values table
    – Vatsal
    Commented Jun 11, 2014 at 20:07
  • @Vatsal, Oh, sorry! My mistake! Edited!
    – kzidane
    Commented Jun 11, 2014 at 20:18
printf("%c",((plaintext[i] - 65) + (word[j % strlen(word)])) %26 + 65);


printf("%c",((plaintext[i] - 97) + (word[j % strlen(word)])) %26 + 97)

this might be where your problem is. you need to first set the int value of the characters in word, with A and a as 0 etc, like you do with the plaintext.

some other things: it can be easier to write -'a' rather than -97, in case you forget the ascii value. also, in your for loops, e.g.

for (int i = 0; i < strlen(plaintext); i++)

it might be better to declare another int with a value equal to strlen(plaintext), so that function doesn't have to be executed with every loop, just once at the beginning, e.g.

for (int i = 0, n = strlen(plaintext); i < n; i++)
  • Thanks! I am seriously going to consider using the letters instead of the ASCII values.
    – emece
    Commented Jun 11, 2014 at 15:13

You must log in to answer this question.

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