-1

This is a Vigenere cypher for cs50. It's my first time coding and I'm going around this for a week now, and I don't seem to be able to print first letter after the loop finishes for the first time.

For example:

jharvard@appliance (~/Dropbox): ./viginere abcde 
You're key is abcde
Type your text: 
aaaaa aaaaaa aaaaaaa aaaaaaaa
abcde bcde bcdebc debcde

First a is printed but then it starts on b and in the end it doesn't print every letter. The key is chosen by the user.

I have no idea what I'm doing wrong.

for (int i = 0, j = strlen(plain_text), l = 0; i < j; i++)
{    
    int rotation_1 = (tolower(plain_text[i]) + (key[l] - 97)) % 122;
    int rotation_2 = (plain_text[i] + (key[l] - 97)) % 122;

    //if it is a letter
    if (isalpha(plain_text[i]))
    {   
        l = l % strlen(key);
        //if the it is uppercase
        if (isupper(plain_text[i]))
        { 
            printf("%c", toupper(rotation_1)); 
        }
        //else if it is lowercase
        else
        {  
            printf("%c", rotation_2);   
        }
        l++;        
    }                     
    // if it is not a letter we print it as it is     
    else           
    {
        printf("%c", plain_text[i]);
    }            
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Bartolomeu
  • 1
  • 1
    Don't use `l` as a variable name, it looks confusingly similar to `1`. – chqrlie Apr 05 '15 at 18:56
  • Similar question http://stackoverflow.com/questions/29319393/vigenere-cipher-black-hawk-down – Barmak Shemirani Apr 05 '15 at 18:57
  • You should update `l` this way: `l = (l + 1) % strlen(key);` at the end of the `if` branch and remove the update code at the beginning. – chqrlie Apr 05 '15 at 19:02
  • The logic is faulty. The way you compute `rotation_1` and `rotation_2` does not yield letters in the proper range. You should compute the letter index, adjust it using the key, take the modulo, and finally add the base letter (`'A'` for uppercase, `'a'` for lowercase). – chqrlie Apr 05 '15 at 19:09
  • OK I think I get it.. I won't use "l" anymore, but even tho I'm not computing words correctly for some examoles it works, ill to break rotation_1 to simples and easier parts and see if I can follow your advice – Bartolomeu Apr 05 '15 at 19:19

1 Answers1

2

Here is a simpler version with the logic fixed:

for (int i = 0, n = strlen(plain_text), k = 0, klen = strlen(key); i < n; i++) {
    int c = (unsigned char)plain_text[i];

    //if it is a letter
    if (isalpha(c)) {
        if (isupper(c)) {
            c = 'A' + (c - 'A' + key[k] - 'a') % 26;
        } else {
            c = 'a' + (c - 'a' + key[k] - 'a') % 26;
        }
        k = (k + 1) % klen;
    }
    putchar(c);
}

It is important to cast plain_text[i] as (unsigned char) because isalpha and isupper are only defined for all values of unsigned char and EOF. char may be signed by default of your platform.

Also note that putchar(c) is much more efficient than printf("%c", c);

chqrlie
  • 131,814
  • 10
  • 121
  • 189