Caesar Cipher: Correct encryption but output missing spaces and punctuations

Question

I have been working on this since yesterday and after much struggle have managed to encrypt the message successfully. However, my output is missing spaces.

As I understand it, the reason this is happening is because I'm using isalpha(), isupper() and islower() commands and as a result am ignoring the spaces in the original input.

Can one please help me with how to retain the original spaces and punctuation?

Below is my code- its far from elegant and any comments on style will also be appreciated!

(Also, while there are plenty of questions on Caesar Cipher, none barring one deal with this problem. Since this is my first week programming, I had trouble understanding the syntax in that one.)

There is a blatant mistake in my algorithm which causes it to output the wrong values if given certain arguments. For instance with a k of 13, inputting anything after the 13th alphabet (m, I think) will output something very bizarre. I will amend this and get back soon! Till then, take my code with a grain of salt!

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


int main(int argc, string argv[])

{
    if (argc != 2)
    {
        printf("Please enter a valid number of arguments! \n");
        return 1;
    }

    string num = argv[1];
    int k = atoi(num);

        if (k < 0)
            {
                printf("Please enter a valid number! \n");
                return 1;
            }

    printf("Please type the message which needs to be encrypted: ");
    string p = GetString();

        for (int i = 0, n = strlen(p); i < n; i++)
        {
            int oldletter = p[i];
            int result1 = (oldletter + k);
            int result2 = (oldletter - 65 + k);
            int result3 = (result2) % 26;
            int result4 = (oldletter - 97 + k);
            int result5 = (result4) % 26;

                if (isalpha(p[i]) && isupper(p[i]) && k < 26)
                {
                    printf("%c", result1);
                }

                if (isalpha(p[i]) && isupper(p[i]) && k >= 26) 
                {
                    int result7 = (result3 + oldletter); 
                    printf("%c", result7);
                }


                if (isalpha(p[i]) && islower(p[i]) && k < 26)
                {
                    printf("%c", result1);
                }

                if (isalpha(p[i]) && islower(p[i]) && k >= 26)
                {
                    int result8 = (result5 + oldletter); 
                    printf("%c", result8);
                }

        }
        printf("\n");
}

Corrected code, working properly: SPOILERS AHEAD

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

int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Please enter a valid number of arguments! \n");
        return 1;
    }

    string num = argv[1];
    int k = atoi(num);

    if (k < 0)
        {
            printf("Please enter a valid number! \n");
            return 1;
        }

    printf("Message: ");
    string p = GetString();

        for (int i = 0, n = strlen(p); i < n; i++)
        {
            int oldletter = p[i];
            int result1 = (oldletter - 65 + k);
            int result2 = (result1) % 26;
            int result3 = (oldletter - 97 + k);
            int result4 = (result3) % 26;


                if (isalpha(p[i]) && isupper(p[i])) 
                {
                    int result5 = (result2 + 65); 
                    printf("%c", result5);
                }


                else if (isalpha(p[i]) && islower(p[i]))
                {
                    int result6 = (result4 + 97); 
                    printf("%c", result6);
                }

                else 
                {
                    printf("%c", p[i]);
                }

        }
        printf("\n");
}

Answer 1

You should probably chain together the if's as else if's, there is no need to evaluate an if condition if one of the previous was already true in your case. This would also allow for a final else case to be executed when isalpha is false like in the case with spaces.

Just change the if conditions to:

if (isalpha(p[i]) && isupper(p[i]) && k < 26)
{
    printf("%c", result1);
}

else if (isalpha(p[i]) && isupper(p[i]) && k >= 26) 
{
    int result7 = (result3 + oldletter); 
    printf("%c", result7);
}


else if (isalpha(p[i]) && islower(p[i]) && k < 26)
{
    printf("%c", result1);
}

else if (isalpha(p[i]) && islower(p[i]) && k >= 26)
{
    int result8 = (result5 + oldletter); 
    printf("%c", result8);
}

else
{
    printf("%c", p[i]);
}

---

I wanna note that your logic is pretty complex and you should also choose better names than the result* variables that you currently use, in programming readability and maintainability are really important. You can easily do assignments without considering them due to the small programs you write there but it's a good habit to get into.

I also took the course (with prior C experience) and uploaded my final solution for you to compare to/improve with once you finished yours. Just a warning, I used a function, not sure if it was already explained before this problem set but should be at least explained soon after. Here is my solution: http://pastebin.com/vJqPY6Ne

Answer 2

A common pitfall I see when people implement this is to go straight to ascii values. Consider making an alphabet array, you can just get the position of your current letter in it, and then determine what the modified letter should be.

Imagine adding a '%' character to this with the ascii solution, you'll end up having a ton of special if's. You can chose in that case to ignore spaces/etc if you like, personally I would add them to the alphabet array so the ciphertext didn't reveal the spaces (giving away hints).