-1

My program below conducts a Caesar's cipher in C. For some reason, after a message has been input by the user, the printf(" \nEnter key:") and scanf("%d", &key) statements get "jumped" over. My thought was something related to the input buffer having a char and newline entered causing the jump (hence the fflush attempt). How do I prevent this behavior?

#include <stdio.h>
#include <stdlib.h>

int main() {
    char message[50], ms;
    int i, key, choice;

    printf("Enter 1. to Encrypt, or 2. to Decrypt: ");
    scanf(" %d", &choice);

    printf("Enter a message to process: ");
    scanf(" %c", message);
    printf(" \nEnter key:");
    fflush(stdin);
    scanf("%d", &key);

    for (i = 0; message[i] != '\0'; ++i) {
        ms = message[i];
        if (ms >= 'a' && ms <= 'z' && choice == 1) {
            ms = ms + key;
            if (ms >= 'a' && ms <= 'z' && choice == 2) {
                ms = ms - key;
                if (ms > 'z') {
                    ms = ms - 'z' + 'a' - 1;
                }
            }
            message[i] = ms;
        } else
        if (ms >= 'A' && ms <= 'Z' && choice == 1) {
            ms = ms + key;
            if (ms >= 'A' && ms <= 'Z' && choice == 2) {
                ms = ms - key;
            }
            if (ms > 'Z') {
                ms = ms - 'Z' + 'A' - 1;
            }
            message[i] = ms;
        }
        if (choice == 1) {
            printf(" \nEncrypted message: %s", message);}
        else if (choice == 2) {
            printf(" \nDecrypted message: %s", message);}
    }
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
ddisec
  • 71
  • 1
  • 6
  • 4
    `fflush(stdin)` is never correct. `fflush` is for output, not input. – aschepler May 27 '18 at 18:31
  • When running, I get this output: `Enter 1. to Encrypt, or 2. to Decrypt: 1 Enter a message to process: hello Enter key:1 Encrypted message: i`. It appears that it's not skipped over for me, but there may be other issues. – Stephen P May 27 '18 at 18:33
  • Following the comment 2 back `scanf(" %c", message);` is incorrect. `message` is an array, decaying to a pointer, not a `char`. Did the compiler warn you? Mine did not but until you correct that the output is absurd. – Weather Vane May 27 '18 at 18:33
  • scanf is not a great function, because of its unpredictability in consuming input on failure to perform a conversion. Consider using fgets to read input line-by-line, followed by sscanf, and also checking the return values of standard library functions. – tallungulate May 27 '18 at 18:41
  • @WeatherVane if I instead use gets(message) for example, then my printf's prompting for both the message and the key print with no waiting for user input. – ddisec May 27 '18 at 18:46
  • 1
    `gets` is dangerous and obsolete. Please use `fgets`. Don't mix the methods. Use `fgets` and then `sscanf` on its input. – Weather Vane May 27 '18 at 18:48
  • You need to get a book on the "C" language and study it. You need to understand each line of code you write. You need to understand ASCII binary encoding and values. – zaph May 27 '18 at 19:32

2 Answers2

1

@ddisec I have noted 3 mistakes in your code .

First your scanf(" %c", message); .Here you have to use %s (String).

Second the result printing statements should be outside for-loop.

Third putting if(ms >= 'a' && ms <= 'z'&& choice == 2) inside if (ms >= 'a' && ms <= 'z' && choice == 1) dose not make any sense.

Try this corrected code:-

#include <stdio.h>
#include <stdlib.h>

int main()
{
    char message[50], ms;
    int i, key, choice;

    printf("Enter 1. to Encrypt, or 2. to Decrypt: ");
    scanf("%d", &choice);
    getchar();                                              // to handle unwanted newline.
    printf("Enter a message to process: ");
    scanf("%49[^\n]", message);
    printf(" \nEnter key:");
    scanf("%d", &key);

    for (i = 0; message[i] != '\0'; ++i)
    {
        ms = message[i];
        if (ms >= 'a' && ms <= 'z' && choice == 1)
        {
            ms = ms + key;
        }
        else if (ms >= 'a' && ms <= 'z' && choice == 2)
        {
            ms = ms - key;
        }
        else if (ms > 'z')
            {
                ms = ms - 'z' + 'a' - 1;
            }
        else if (ms >= 'A' && ms <= 'Z' && choice == 1)
        {
            ms = ms + key;
        }
        else if (ms >= 'A' && ms <= 'Z' && choice == 2)
        {
            ms = ms - key;
        }
        else if (ms > 'Z')
        {
            ms = ms - 'Z' + 'A' - 1;
        }

        message[i] = ms; // Only single modification code needed.
    }
    if (choice == 1)
    {
        printf(" \nEncrypted message: %s", message);
    }
    else if (choice == 2)
    {
        printf(" \nDecrypted message: %s", message);
    }
}
anoopknr
  • 3,177
  • 2
  • 23
  • 33
  • Thanks! I did notice my printf's at the end being inside the loop. Having the choice == 2 statement inside of choice == 1's was a silly mistake on my part. Still learning a lot as I go! – ddisec May 27 '18 at 19:59
  • You should also consider `ms < 'a'` when decrypting and we're in lower-case mode, or likewise `ms < 'A' ` in upper case, and add 26 if needed. `If (ms > 'Z') ms-=26` makes more sense to me. – Henno Brandsma Jun 01 '18 at 07:39
0

There are multple problems in your code:

  • The way you enter the message is incorrect: scanf(" %c", message); reads a single byte into the message first element and does not even make it a C string. Use this instead, to read the message with embedded spaces:

    scanf("%49[^\n]", message);
    
  • There are logic bugs in the rest of the code: for example, you test for choice == 2 inside the block that is only executed if choice == 1...

Here is a simplified version:

#include <stdio.h>

int main() {
    char message[50];
    int i, key, choice;

    printf("Enter 1. to Encrypt, or 2. to Decrypt: ");
    if (scanf("%d", &choice) != 1)
        return 1;

    printf("\nEnter a message to process: ");
    if (scanf("%49[^\n]", message) != 1)
        return 1;
    printf("\nEnter key:");
    if (scanf("%d", &key) != 1)
        return 1;

    for (i = 0; message[i] != '\0'; ++i) {
        int ms = message[i];
        if (ms >= 'a' && ms <= 'z') {
            if (choice == 1)
                ms = 'a' + ((ms - 'a') + key) % 26;
            if (choice == 2)
                ms = 'a' + ((ms - 'a') + 26 - key) % 26;
            message[i] = ms;
        } else
        if (ms >= 'A' && ms <= 'Z') {
            if (choice == 1)
                ms = 'A' + ((ms - 'A') + key) % 26;
            if (choice == 2)
                ms = 'A' + ((ms - 'A') + 26 - key) % 26;
            message[i] = ms;
        }
    }
    if (choice == 1)
        printf("\nEncrypted message: %s\n", message);
    if (choice == 2)
        printf("\nDecrypted message: %s\n", message);
    }
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Nice, this does seem far simpler. However, this code doesn't seem to work when I build/run in VS17. – ddisec May 27 '18 at 19:51
  • @ddisec: try this modified version with `ms` defined as `int`. What is the error message? – chqrlie May 27 '18 at 21:26