Print Caesar Shift in C is printing the entered text of HELLO

Question

I am trying to perform a Caesar cipher from text from user using modulo operation with the ascii characters. But, my code simply prints the entered test. For example, when the text entered is HELLO the program returns "HELLO". The goal is for a key of 13 it should print URYYB. Thank you.

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>
int key = atoi(argv[1]);

string plaintext = get_string("Enter plaintext: ");

for (int i = 0; i < strlen(plaintext); i++)
{
    if (isalpha(plaintext[i]))
    {
        if (isupper(plaintext[i]))
        {
            printf("%c", ((plaintext[i] + key) % 26) + 65);
        }
        else if (islower(plaintext[i]))
        {
            printf("%c", ((plaintext[i] + key) % 26) + 97);
        }
        else
        {
            printf("%c", plaintext[i]);
        }
    }
}
printf("\n");

`atoi` is not a particularlly good function as it does not indicate an error i.e. if the string is not a number — Ed Heal, Dec 22 '18 at 11:29
`string` is that some typedef for `char*` ? probably good to show your get_string function, i suspect you return something that no longer exists, then you get undefined behavior. — AndersK, Dec 22 '18 at 11:35
duplicate of [Character Shift returning same as original character in C](https://stackoverflow.com/q/53893954/995714) — phuclv, Dec 22 '18 at 11:40
Unrelated to the actual problem, I recommend iterating until you find the terminating NUL byte (zero, `'\0'`) of the string instead of using `strlen`. (In this case you are also re-calculating the length of the string on every iteration in theory, which is a bad habit worth getting rid of, even if in this kind of simple program it is insignificant and potentially optimised away by the compiler.) — Arkku, Dec 22 '18 at 12:09
you don't have a main function. `int key = atoi(argv[1]);` doesn't work at file level — phuclv, Dec 24 '18 at 13:32

score 2 · Accepted Answer · answered Dec 22 '18 at 12:34

Preliminary analysis

Character code of 'H' is 72.

(72 + 13) % 26 + 65 = 85 % 26 + 65 = 7 + 65 ~ 'H'

Let's see if we subtract 65 first:

(72 - 65 + 13) % 26 + 65 = (7 + 13) % 26 + 65 = 20 % 26 + 65 = 20 + 65 = 85 ~ 'U'

Solution

printf("%c", ((plaintext[i] + key - 65) % 26) + 65);

and

printf("%c", ((plaintext[i] + key - 97) % 26) + 97);

respectively.

Proof

If you have a character code, C, where S <= C < S + 26, then the formula you used is:

((C + key) % 26) + S

however, the actual letter is L and we know that

C = S + L,

so the formula is

((S + L + key) % 26) + S

and, since

(A + B) % C = ((A % C) + (B % C)) % C,

replacing A with (S), B with (L + key) and C with 26, we get:

((S % 26) + ((L + key) % 26)) % 26, we see that the result is distorted by (S % 26), which, in the case of 65 is exactly 13. Since a distortion of 13 + the key of 13 you used in the modulo class of 26 will yield the initial letter!

So, the proposed new formula of

((C + key - S) % 26) + S = (((S + L) + key - S) % 26) + S = ((L + key) % 26) + S

is exactly what you need.

score 2 · Answer 2 · answered Dec 22 '18 at 12:36

2

You are adding key to the value of each plaintext character, when it is meant to apply to the corresponding letter's index in the alphabet. For example, in case of the 'H' in ASCII, your formula is: (72 + 13) % 26 which gives 7 (which is also the index of H in the alphabet, when starting from zero).

You need to convert the (ASCII) value of the character to its index before applying key, e.g., ((plaintext[i] - 'A' + key) % (1 + 'Z' - 'A')) + 'A'.

The solution for 'H' would then become (72 - 65 + 13) % 26, which gives 20 (the correct answer, 7 + 13, the index of U).

answered Dec 22 '18 at 12:36

Arkku

41,011
10
62
84

I upvoted your answer, as it is correct, but I had given the same solution earlier :) – Lajos Arpad Dec 22 '18 at 12:40
@LajosArpad Well, 1 min difference means we were both typing at the same time, but I also upvoted yours (although I would prefer substituting the magic number `65` with `'A'`). – Arkku Dec 22 '18 at 12:43
I know you were not copying mine, it would have been impossible in this amount of time and even otherwise I would not assume such thing. I was just pointing out a funny fact. As about replacing 65 with 'A' makes sense, however, I used the 65 because the asker used it as well. However, I agree that 'A' is more readable for the human eye. – Lajos Arpad Dec 22 '18 at 12:45
@LajosArpad _In theory_ the program could also be run with a non-ASCII-compatible character set, in which case 65 would be incorrect while `'A'` might still work (if the letters of the alphabet were still contiguous and in order). =) – Arkku Dec 22 '18 at 12:47
Good point, however, that would be outside the scope of the specification we can read in the question. – Lajos Arpad Dec 22 '18 at 12:49

score 0 · Answer 3 · answered Dec 22 '18 at 11:37

Your cipher function just does nothing if the key is 13:

run a bit amended one and see the result :D

int main()
{
    int key = 13;

    char  plaintext[] = "HELLO";

    for (int i = 0; i < strlen(plaintext); i++)
    {
        if (isalpha(plaintext[i]))
        {
            if (isupper(plaintext[i]))
            {
                printf("%d, %d\n", (int)plaintext[i], (int)(((plaintext[i] + key) % 26) + 65));
            }
            else 
            {
                if (islower(plaintext[i]))
                {
                    //printf("%c", ((plaintext[i] + key) % 26) + 97);
                }
                else
                {
                    //printf("%c", plaintext[i]);
                }
            }
        }
    }
    printf("\n");
    return 0;
}

Print Caesar Shift in C is printing the entered text of HELLO

3 Answers3

Preliminary analysis

Solution

Proof