Vigenere Cipher Doesn't Work Properly

Question

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;

Answer 1

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:

Capital

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.

Cheers!

Answer 2

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.

Answer 3

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.

Answer 4

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

and

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++)