Caesar cipher outputs blank line

Question

I am doing a problem set from the CS50 course and we have to implement Caesar's cipher. The following code works only with numbers (they remain the same as intended), when you put in a character, however, nothing is output. What's wrong?

#include <cs50.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>
//problem set requires arguments
int main (int argc, string argv [])
{
    int i = 0;

    if (argc != 2){
        printf ("Retry\n");
        return 1;
    } else {
        int x = atoi(argv [1]);
        //gets plaintext
        string a = get_string ("plaintext:");
        printf("ciphertext:");
        for (i = 0; i <= strlen(a); i++){
            //if character is a number it remains unchanged
            if (isdigit(a[i])){
                printf ("%c", a[i]);
            } else {
                if (isupper(a[i])){
                    //converts from ASCII index to alphabetical index
                    char y = a[i] - 65;
                    //Caesar cipher formula. If upper case remains upper case.
                    y = toupper(a[i] + x % 26);
                    //goes back ASCII
                    printf("%c", y + 65);
                    } else if (islower(a[i])){
                    //converts from ASCII index to alphabetical index
                    char t = a[i] - 65;
                    //Caesar cipher formula. If lower case remains lower case.
                    t = tolower(a[i] + x % 26);
                    //goes back to ASCII
                    printf("%c", t + 65);
                    }
                }
           }
        }
}

Why `y = toupper(..`? It already is an uppercase letter per the above `if`, and it's (at the same time) *not* any letter because you already subtracted 65 from it. — Jongware, Jan 14 '18 at 01:31
Without looking it up: I would think that I'd rather use some parentheses to make `a = b + c % d` do *exactly* what I mean, rather than relying on precedence. It also makes it easier to guess what you do for someone else who reads your code. — Jongware, Jan 14 '18 at 01:33
Where is `get_string()` defined? According to the version I have, it's NOT defined in `cs50.h`. — Mark Benningfield, Jan 14 '18 at 02:19
There are over 300 questions tagged [tag:caesar-cipher], many of them also tagged [tag:cs50]. Have you looked among them for answers to your problems? Your code is hard to read. The indentation and bracing of the second `else if` doesn't match the first. I'm not clear how you handle non-alphanumeric characters (AFAICT, you skip them completely). — Jonathan Leffler, Jan 14 '18 at 03:12
@MarkBenningfield: There was a major revision to the `cs50.h` and `cs50.c` code a year or two (or three?) ago, and names like `get_string()` were added then (supplanting the older `GetString()`). The new code does some interesting things, like managing the release (freeing) of returned strings. It is no longer a library of code that the students of CS50 could be expected to write — whereas the old version was more or less one which they could have written at the end of the course. — Jonathan Leffler, Jan 14 '18 at 03:15
@JonathanLeffler: Yeah, that figures. That's about how old my version is (3.0). — Mark Benningfield, Jan 14 '18 at 17:47
@AdamGrey: Are you sure this is the code that doesn't work? After updating my version of cs50.h, I cannot reproduce your problem. — Mark Benningfield, Jan 14 '18 at 18:06
@MarkBenningfield I just checked again and yes, this code doesn't work. How can I update a library? Did you have my problem before updating? — Adam Grey, Jan 14 '18 at 20:39
@AdamGrey: No, I didn't. By the simple expedient of swapping out the `get_string()` function with separate calls to the older (now deprecated) `GetString()` function and `printf()`, the code worked fine. Are you certain your environment can print the resulting characters? (They are in the extended range of the ASCII character set. — Mark Benningfield, Jan 15 '18 at 00:45

Barmak Shemirani · Accepted Answer · 2018-01-14T23:25:45.663

65 is the ASCII value for letter 'A'. If you are working on lower case word, you would need 97, which is ASCII value for 'a'

You have calculated t, but you didn't carry the result to the next line. Moreover, you have to take the modulus on the whole line. You are expecting that t is between 0 and 26, then you add it to 97

char t = a[i] - 97;
t = (t + x) % 26;
printf("%c", t + 97);

Also the for loop should go up to strlen(a). The last index in the string a is null-character, it should not be modified. You can also use 'a' instead of 97 here.

for(i = 0; i < strlen(a); i++)
{
    if(isupper(a[i]))   
    {
        char y = a[i] - 'A';
        y = (y + x ) % 26;
        printf("%c", y + 'A');
    }
    else if(islower(a[i])) 
    {
        char t = a[i] - 'a';
        t = (t + x )% 26;
        printf("%c", t + 'a');
    }
    else 
    {
        printf("%c", a[i]);
    }
}

Caesar cipher outputs blank line

1 Answers1