0

I created this Caesar Cipher program, but for some reason, I can't find its bug. Here is the code. It compiles, but it just prints out the same thing I enter, for any key.

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

string encryptor(string encryptString, int key);

int main(int argc, string argv[]) {
    if (argc >= 2) {
        int k = atoi(argv[1]);
        string stringToBeEncrypted = GetString();
        printf("%s\n", encryptor(stringToBeEncrypted, k));
    } else {
        printf("Please add a key argument\n");
        return 1;
    }
}

string encryptor(string encryptString, int key) {
    for (int i = 0, n = strlen(encryptString); i < n; i++) {
        int ascii = (int) encryptString[i];
        key = key % 26;
        if (isalpha(encryptString[i]) == true) {
            if (islower(encryptString[i]) == true) {
                ascii = ((ascii-96+key) % 26) + 96;
            }else {
                ascii = ((ascii-64+key) % 26) + 64;
            }
            // ascii += key;
            // if (ascii > 122) {
            //     //ascii = ascii - 122 + 96;
            //     ascii = (ascii % 26) + 97;
            // } else if (ascii > 90 && ascii < 97) {
            //     //ascii = ascii - 90 + 64;
            //     ascii = (ascii % 26) + 65;
            // }

        }

        char newChar = (char) ascii;
        encryptString[i] = newChar;
    }
    return encryptString;
}
1

The problem lies with your use if isalpha() and islower(). Your code is:

if (isalpha(encryptString[i]) == true)

Surprisingly unintuitively, if the character is an alpha, this will NOT evaluate as a true statement.

If you read the documentation, it says that it returns 0 if false and a non-zero if true. This number varies, depending on the architecture, OS, and other factors. (In the cloud9 IDE, it's 1024.) The fail occurs when you compare that number to true. However, all architectures treat 0 as false and non-0 as true, even though it doesn't compare equally to true. So, either of the following will work, although the first is preferred.

if ( isalpha(encryptString[i]) )

if (isalpha(encryptString[i]) != false)

Same logic applies to isupper and islower.

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

3
  • That is extremely weird, why didn't they just make true and false? – Epic Defeater Apr 16 '16 at 22:28
  • When I set both of isalpha and islower to 1024(I am using the cloud 9 IDE) it still does the same thing. Also, when I read the documentation, it said that 1 is true. I also tried that, but it also didn't work. Your suggestions both worked though. Thanks! – Epic Defeater Apr 16 '16 at 22:44
  • Yeah, well, that's another thing. Occasionally, and depending on the version of the docs, the version of the code implementation, if the two are mismatched, and whether Mercury is in retrograde, the documentation can be wildly wrong. ;-) That's why the first example above is preferred. Don't compare the value returned to true/false/number, just let it stand as true or false in itself. It's much more reliable. It's also slightly less code and slightly more efficient. (It eliminates one evaluation.) – Cliff B Apr 16 '16 at 22:48

You must log in to answer this question.

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