my vigenere program isn't doing quite what it's supposed to so I was wondering if anyone could tell me if my formula is correct. I don't need the answer, just which part (if any) of the code I post is wrong. Thanks!

for (int i = 0, pl = strlen(plaintext), kl = strlen(argv[1]), index = 0 ; i < pl; i++)
    char  current = plaintext[i];

        if (islower(current))
            int k = argv[1][index % kl] - 97;
        current = (current + k) % 26;
        current = current + 97;

  • When you have posted only a few lines of code, there's no need to edit it out later. It makes your question no longer useful to other people, and if someone wants to copy your code they can still read it in the edit history of the post.
    – Air
    Commented Jul 2, 2014 at 18:46

2 Answers 2


First, you should consider ciphering uppercase characters too!

Second, the formula assumes that each of your alphabetical characters has a value in [0, 25].
For example, when ciphering, the letter A should be equivalent to 0, the letter B should be equivalent to 1 ... Z should be equivalent to 25.
Similarly, a should be equivalent to 0, b should be equivalent to 1 ... z should be equivalent to 25.

At the same time, after ciphering, each character should have its original ASCII value back.
For example, if the original char was ciphered to A (i.e., 0), it should become 65. If, rather, it was ciphered to B (i.e., 1), it should become 66 and so on.

Same thing applies for lowercase chars.

Third, the ciphering formula per the pset specification page is

c = (p + k) % 26

Where c is the ciphered char, p is the current plain char in [0, 25] and k is the current key. And since k must be an alphabetical char, it has to be in [0, 25] either.

What you're doing wrong is

  1. You're never getting the value of your plain char (i.e., current) to be in [0, 25].

  2. You're not considering uppercase chars either in the plaintext or in the keyword.

  3. You're adding 97 to current at the end without subtracting 97 from it at the first place.

  • I did have a scenario for upper case, I just didn't post it because it was the same equation with different numbers. sorry for the confusion.
    – Sofia
    Commented Jul 2, 2014 at 16:36
int k = argv[1][index % kl] - 97;

What if argv[1][0] is 'B'? Do you want k to be ('B' - 97)?

Shouldn't you be zero-basing your key? So that 'A' = 0? 'B' = 1? etc..

Your code will have 'A' = -32. Make sure you consider that the argv[1] can include both upper and lower case characters.

  • that is why I have if( is lower). I have a separate formula for is upper where I use 65 instead of 97. I just didn't post it.
    – Sofia
    Commented Jul 2, 2014 at 5:23
  • You have "if (islower(current))" which tests whether the plaintext is lower. That has no bearing on whether the keyword is lower.
    – curiouskiwi
    Commented Jul 2, 2014 at 6:46

You must log in to answer this question.

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