why is my vigenere giving the wrong encryption for upper case letters?

I was having issues with my for loop, but with the help and guidance of

fellow coders in here I was able to fix most of it.

I managed to do the caesar cipher and I used it as a frame of reference.

Now my code is working when it's lower case but I'm having issues with the upper case condition

here is my updatedcode:

for ( int i = 0 , j = 0 ; i < strlen(text) && j < key[j % strlen(key)] ; i++, j++ ){

// check if it's lower case
if (islower(text[i])) {
printf("%c", ((text[i] - 'a' + key[j] - 'a') % 26 + 'a'));
j++;
if (j == strlen(key)){
j = 0;

}

// check if it's upper case
} else if (isupper(text[i]))  {
printf("%c", ((text[i] + 'A' + key[j] + 'A') % 26 + 'A'));
j++;
if (j == strlen(key)){
j = 0;

}
}

When I compile my program and give it :

./vigenere bacon

Meet me at the park at eleven am

Negh zf av huf pcfx bt gzrwep oz

I get this wrong answer with everything is correct except the 1st uppercase encryption:

Tegh zf av huf pcfx bt gzrwep oz

There are a couple of obvious problems. First, look at your formula for encoding a letter. You correctly figured out that the letter to be encoded has to be converted from it's ascii value to a number between 0 and 25. So what do you think happens when you add the key letter's ASCII value without doing the same shift? Let's look at your code and work through the second letter from your example:

((text[i] - 'a' + key[j]) % 26 + 'a'));

Using key = a = 97 , text = e = 101

(( 'a' - 'a' + 'e' ) % 26 + 'a'
= (( 97 - 97 + 101 ) % 26 + 97
= (( 101 )) % 26 + 97
= 23 + 97
= 120
= 'x'

which is exactly what the code came up with. Next, the formula for upper case letters has additional flaws.

Because of the setup of the for loop, the code increments the indexes for both the plain text and for the key. This will cause the encoding to go bad when the first non-alpha is processed. The key index is ONLY incremented when a letter is actually encoded. It's much easier and better to just use a simple variable for the key index, increment it and reset it to zero when appropriate. (I'm not even sure why you are comparing the value of j to the ASCII value of key[j] in the for loop setup.)

As a side note, look at your two tests. Each tests for isalpha() and then for either isupper() or islower(). The isalpha() part of the tests is unnecessary. For isupper() or islower() to return true, isalpha() must be true. If something isn't an alpha, both isupper() and islower() will return false.

This should get you going. If this answers your question, please click on the check mark. Let's keep up on forum maintenance. ;-)

• Your guidance helped me, but I'm still confused on how to set up my for loop. If I understood correctly, I shouldn't increment j on every loop but instead every time I use the key[j]. Could you please elaborate on that. Tks. Jan 8 '16 at 18:34
• After some modification I now get some correct result on some characters Jan 8 '16 at 18:35
• It's not that difficult. Create a variable, say j. Every time you encode a letter, add 1 to it. When it gets large enough, reset it to 0. And take it out of the for loop setup. Jan 8 '16 at 19:18
• Yes I managed to do it, now it's working except when it's an uppercase letter. So basically everything is encrypted correctly except the first upper case letter. I guess I have to tweak my code located in the isupper condition. Jan 8 '16 at 19:35
• Your formula for encoding uppercase is flawed. Compare it to the formula for lower case and it should be obvious. Jan 8 '16 at 19:43