Caesar cipher in C working for lower not upper

Question

Cipher works for islower portion but not isupper portion. For instance if I give a key of 3 and enter I like pie!! to be encrypted, I get O olnh slh!! I also tried HELLO and got NKRRU. The isupper portion is also returning punctuation instead of just letters. I also have not figured out why the original message is being altered to match the cipher message.

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

int main (int argc, string argv[])
{
    /*
    Get key from user at command line
    Get plaintext from user
    Use key to encipher text: c[i] = (p[i] + k)%26
    Print ciphered message
    */        
    string message, cipher;
    int key;

    // command line, if user doesn't enter 2 arguments return 1 and request a valid 
    //encryption key and rerun.
    if (argc != 2)
        {
        printf("Please enter a valid encryption key and rerun program.\n");
        return 1;
        }
    else
        {
        key = atoi(argv[1]);
        }


    printf("Enter the message you wish to encrypt.\n");
    message = GetString();
    cipher = message;
    int length = strlen(message);

    for ( int i = 0; i < length; i++)
        {
        if (isalpha(message[i]))
            {
            if (isupper(message[i]))
                {
                cipher[i] = (message[i] - 'A' + key) % 26 + 'A';
                }
            else (islower(message[i]));
                {
                cipher[i] = (message[i] - 'a' + key) % 26 + 'a';
                }
            }
        else continue; //message[i] contains punctuation or a space
        } 
         printf("Your original message was..\n");
         printf("%s\n", message);
         printf("The encrypted message is...\n");
         printf("%s\n", cipher);         
         return 0;            
}

chux - Reinstate Monica · Answer 1 · 2014-02-09T19:44:09.133

3

Typo and missing if per @interjay.

Change

else (islower(message[i]));

to

//                           v
else if (islower(message[i]))
// or simply 
else  // Since `message[]` is an alpha, but not upper

With the error, when text was uppercase, both cipher[i] = (message[i] - 'A' ... and cipher[i] = (message[i] - 'a' ... occurred. Given cipher = message, the cipher was applied twice.

@keshlam point about the missing buffer is a significant issue. But I wonder what type string is. Is this some sort of C++ lite string? If it is a char *, code could use cipher = strdup(message); or

cipher = malloc(length + 1);
if (cipher === NULL) Handle_OutOfMemeory();
cipher[length] = '\0';
for ( int i = 0; i < length; i++) 
  ...

edited Feb 09 '14 at 19:44

answered Feb 09 '14 at 19:24

chux - Reinstate Monica

143,097
13
135
256

You need to add an `if` there as well. – interjay Feb 09 '14 at 19:25
I'm assuming string is typedef'd as `char*`. Not good practice, but I haven't been happy with a lot of the curricula as implied by the questions we're getting here. – keshlam Feb 09 '14 at 19:33

score 2 · Answer 2 · answered Feb 09 '14 at 19:24

2

You're overwriting message because you said cipher = message; which means both now point to the same block of memory. Allocate a new output buffer.

And two points to Chux for spotting the superfluous semicolon.

answered Feb 09 '14 at 19:24

keshlam

7,931
2
19
33

Caesar cipher in C working for lower not upper

2 Answers2

Linked