I'm stuck at PSet 2 Caesar Cipher: The alphabets don't follow my modulo 26. It goes beyond it, printing non-alphabetic characters

Question

The crux is at the last part where I've commented "CONVERT PLAINTEXT TO CIPHERTEXT" The variable asciiValue holds the, well, the asciiValue of the enciphered character. As you can see, I've used the modulo to not let it overflow to anything other than alphabets.

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

int main (int argc, string argv[])
{
    //Create int key to be initialized later.
    int key;

    ////////////////////////////////////
    //CHECKING FOR A VALID KEY PASSED IN

    //Check if 2 arguments passed in
    if (argc != 2)
    {
        printf("One command line argument allowed\n");
        return 1;
    }
    else
    {
        //Check if all the characters are digits
        for (int i = 0, stringLength = strlen(argv[1]) ; i < stringLength ; i++)
        {
            if (!isdigit(argv[1][i]))
            {
                printf("Please pass in a valid INTEGER\n");
                return 1;
            }
        }

        //If an integer passed as key,
        //convert it to int and store in variable key
        key = atoi(argv[1]);

        //If key is negative, return 1
        if (key < 0)
        {
            printf("Key has to be a POSITIVE INTEGER\n");
            return 1;
        }

    }


    ////////////////////////////////////
    //GETTING PLAINTEXT FROM USER

    string plaintext = get_string("Plaintext: ");


    ////////////////////////////////////
    //CONVERT PLAINTEXT TO CIPHERTEXT

    printf("Ciphertext: ");

    for (int i=0, stringLength = strlen(plaintext) ; i < stringLength ; i++)
    {
        if (isalpha(plaintext[i]))
        {
            int asciiValue = plaintext[i]+key%26;
            printf("%c",asciiValue);
        }
        else
        {
            printf("%c", plaintext[i]);
        }
    }

}

And on another side, I'm still confused why did I even use isalpha() function instead of isupper() and islower() since they both exist at different parts of the ASCII table. But it still does make a little sense especially since the code works fine, except it prints out non-alphabetic characters when it passes z/Z.

Answer 1

The problem lies here:

int asciiValue = plaintext[i]+key%26;

Simply put, it won't work at all. plaintext[i] contains an ASCII value for a letter. That's a number somewhere between 65 and 122 inclusive. Add a key to it and it's even larger. Now, what happens when you apply %26 to it? You ALWAYS get a number between 0 and 25 inclusive. In other words, you always get a non-printable ASCII value.

The reason it doesn't work is that this is an incomplete application of the method. The ASCII value of the letter must first be converted to a number between 0 and 25 inclusive by subtracting 'a' or 'A'. Then, add the key and apply %26. Finally, add back the 'a' or 'A' to convert back to an ASCII value. (If this doesn't make sense, review the shorts for the pset to get a detailed explanation.)

Your instinct about isupper/islower is correct. You would need to use them to determine the case of the plaintext char and to preserve the case in the encoded letter.

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