pset 2 vigenere, key wraparound

Question

I am using the modulo operator on vigenere but not sure where I am going wrong. following is part of my code,

//get plain text

string p = GetString();

//encipher

string k = argv[1];

for (int i = 0, n = strlen(p); i < n; i++)
{         
   if(isalpha(p[i]))
   {            
        int j = 0;
        int m = strlen(k);

        while(j < m)
        {  
            k[j] = ((int) tolower(k[j] - 'a'));

            if(isupper(p[i]))            
            {                     
                int aiu = (int) p[i] - 'A'; //alphabetical index(ai)
                int c = ((aiu + k[j]) % 26); //caesar cipher
                printf("%c", c + 'A'); //back to ascii
            }                          

            if(islower(p[i]))
            {
                int ail = (int) p[i] - 'a';
                int c = ((ail + k[j]) % 26);
                printf("%c", c + 'a');
            }

            i++;
            j = j % m;
            j++;

        }

   }     

   else
   {
        printf("%c", p[i]);
   }
}
   printf("\n");

1). it fails to print special character 2). prints correct cyphertext(ct) for plaintext(pt) equal or smaller than ct. 3). considers numbers as non alpha and prints them as is, only if input(ed) at the beginning of the pt i.e. key = ab pt = 12cd ct = 12ce but pt = cd12 gives ct = ce2 4). few others including makes me feel disappointed(read frustrated) in my abilities.

Answer 1

Although you have most (maybe all) of the pieces, you've created some unnecessarily complex logic. I'll try to discuss them, in no particular order.

k[j] = ((int) tolower(k[j] - 'a')); won't do what you want, but you're close. This code will calculate (k[j] - 'a') first, before trying to apply the tolower() function. Since you're subtracting 'a' from any letter, you're not going to have an ASCII value that represents any letter when you're done, so tolower() will never do anything. A careful rearrangement of parentheses should cure this.

You have the right idea that you need to track p and k with different indexes, i and j. However, your use of both a for loop and a while loop make this needlessly complex. Wouldn't it be simpler to merely increment j after (and only after) a letter is encoded?

Next, you have a couple small bugs in how you handle resetting j to 0.

j = j % m;
j++;

What happens as j approaches m? Let's say m is 3. When j is 2, it drops through and gets incremented to 3 and the code goes into the next loop, so you are going to go past the end of the key string. These two lines should be reversed.

Speaking of j, it is declared and initialized inside the for loop. That means that every time the loop cycles back to the top, j will be set back to 0. It should be initialized before the loop starts. For m, the story is slightly different. Since m is never changed, it doesn't hurt, but it would be far more efficient to do it only once, before the for loop, instead of doing it on every pass through the loop.

There may still be other issues, but this will get you going in the right direction.

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