0

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.

4
  • p.s. I know j++ after j = j % m will not satisfy j<m in the while loop but if I put j++ after j = j % m or make my while(j <= m) , both gives me an error of core dumped. – H shah Sep 4 '15 at 6:26
  • So I made the changes as you suggested and my program can now handle the special characters and all. The issue I am not getting is, although my j increments from 0 to 4 and wraps around, my k[j] is not going back to k[0] as b if bacon is my key. I tried to print out j and corresponding k[j] at the end and it gives me random characters rather than going back to k[0] as b and so on. – @Cliff B – H shah Sep 7 '15 at 9:38
  • I did say there may be other issues. I expected as much. Given the number of changes that you needed to make, there's no way to know what the new problems are without seeing revised code. The answer I gave addressed the question posed, not what new problems there may be. Finally, just a reminder that this is a teaching forum, where the goal is to help you to find the problems, not to solve them for you. It sounds like you're making progress though. If you are unable to resolve all the issues and truly cannot make more progress, feel free to post a new question. Keep at it! – Cliff B Sep 7 '15 at 17:54
  • Sure...I did work on my issues for at least 6 hours including writing the progress of variables on each step and making other changes that I can think of I might be missing. I assure you, that I never intended to make someone solve it for me. I will work again and see if I can solve it or not before posting further. Thank you for being kind. – H shah Sep 8 '15 at 12:47
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. ;-)

1
  • Thank you Cliff B. I can see those mistakes and have better understanding to make my code more efficient now. Still have some issues with the end results but hopefully, will be able to work on them from here. Grateful for your efforts to help. Thank you. – H shah Sep 6 '15 at 12:52

You must log in to answer this question.

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