What can cause a program to ignore the user's input

Question

No matter what option I choose, I'm always prompted to enter a new shift value (as if I entered 1 as my choice). Why does this code ignore the option that I selected?

#include <stdio.h>
#include <string.h>
#define SIZE 500
int getUserChoice()
{
    int decision = 0;

    printf("-------------------------------\n");
    printf("| 1) Change Shift (default 3) |\n");
    printf("| 2) Encrypt a message        |\n");
    printf("| 3) Decrypt a message        |\n");
    printf("| 4) Quit                     |\n");
    printf("-------------------------------\n");
    printf("Option: ");
    scanf("%d", &decision);
    if(decision == 0 || decision >= 4)
    {
        return 0;
    }

    return decision;
}
int getShift()
{
    int key = 3;
    printf("Enter new shift value: ");
    scanf("%d", &key);
   return 0;
}
void getString(char buf[])
{
    int act;
    act = SIZE - 1;
    int count[26] = {0};
    fgets(buf, act, stdin);
    return;
}
void encrypt(char buf[], int shift)
{
    int i = 0;
    for(i = 0; i < strlen(buf);++i)
    {
        buf[i] = buf[i] + shift;
    }
    return;
}
void decrypt(char buf[], int shift)
{
    int i = 0;
    for(i = 0; i < strlen(buf); ++i)
    {
        buf[i] = buf[i] - shift;
    }
    return;
}
int main()
{
    char userStr[SIZE];
    int number = 0;
    getUserChoice();
    int shift = 0;
    if(number = 1)
    {
        getShift();

    }
    if (number = 2)
   {
        getString(userStr);
        encrypt(userStr, shift);
    }
    if (number = 3)
    {
        getString(userStr);
        decrypt(userStr, shift);
    }


    return 0;

}

Answer 1

That looks a lot like homework and you didn't seem to make much effort on finding the issues, but here we go.. There are several flaws on the code:

1) Ignoring return values:

Some of your functions, getUserChoice and getShift, are supposed to return a value but those values are completely ignored in the code. You are not assigning it to any local variables and they are not modifying anything outside their scope, so whatever value is being returned is getting lost.

The code:

if(number = 1){
    getShift();
}

Should be:

if(number == 1){
    shift = getShift();
}

Which brings me to another big mistake, reason of many headaches on C when it happens unintentionally:

2) Mistakenly writing the assignment operator '=' where a comparison operator '==' is required:

The assignment operator returns a value: the left operand value after assignment. So the expression if(number = 1) is valid and will evaluate to true every time, since the returning value of the assignment is always 1. if(number == 1) however, will only evaluate to true when number is equal to 1. I find it useful to make comparisons "Yoda-Style", like if(1 == number) to avoid those kind of mistakes (that can easily happen on a typo). They are hard to track, since the code is valid, but the behavior is not the expected. Using the "Yoda-Style" comparison, you would get a compiler error if you mistyped if(1 = number), since that's an invalid assignment.

3) Not properly filtering the user input:

On the function getUserChoice you have the following conditional statement to check if the input is valid (when invalid it defaults to the "Quit" option):

if(decision == 0 || decision >= 4)
{
    return 0;
}

However, since decision is a signed int, negative values are valid. You don't account for those. The following addresses that:

if(decision <= 0 || decision >= 4)
{
    return 0;
}

4) getShift doesn't return the shift value:

You used scanf on the getShift function to get a shift value, but returned 0. You should return the key value instead.

5) Unused variable at getString + Wrong size argument on fgets:

You declared an array of 0's on getString which serves no purpose on it. Also, from fgets man page:

fgets() reads in at most one less than size characters from stream and stores them into the buffer pointed to by s. Reading stops after an EOF or a newline. If a newline is read, it is stored into the buffer. A terminating null byte ('\0') is stored after the last character in the buffer.

So you don't really need to subtract 1 from SIZE, since it will already account for the null character.

6) Not clearing the stdin before calling fgets:

After the call to scanf("%d",...);, you will have a hanging newline character in your stdin. That's because the format string parsed to scanf there only instructs it to read any number of decimal digits, not including the newline that follows those.

That hanging newline would cause your fgets to quit before getting any user input. You need to clear the stdin by reading the hanging characters from a previous scanf call. In the example, this is being done by this loop:

int c;
while((c = getchar()) != '\n' && c != EOF){
    //Clearing Stdin
}

An alternative is to use:

scanf("%*[^\n]"); scanf("%*c"); //Consume all characters up to \n (if present) then consume exactly one char ('\n' itself)

7) Not using a loop in main to prompt the user repeatedly:

You only prompt the user once, do what is asked and quit, instead of asking the user repeatedly for options until he asks to quit.

8) Unnecessary return statements on the end of void returning functions:

void return functions will return when they reach their end, so there is no need to add a return statement unless you are returning before the end of the function body.

The code below is edited to fix the mentioned issues:

#include <stdio.h>
#include <string.h>
#define SIZE 500
int getUserChoice()
{
    int decision = 0;

    printf("-------------------------------\n");
    printf("| 1) Change Shift (default 3) |\n");
    printf("| 2) Encrypt a message        |\n");
    printf("| 3) Decrypt a message        |\n");
    printf("| 4) Quit                     |\n");
    printf("-------------------------------\n");
    printf("Option: ");
    scanf("%d", &decision);
    int c;
    while((c = getchar()) != '\n' && c != EOF){
        //Clearing Stdin
    }
    if(decision <= 0 || decision >= 4)
    {
        return 0;
    }

    return decision;
}

int getShift()
{
    int key = 3;
    printf("Enter new shift value: ");
    scanf("%d", &key);
    int c;
    while((c = getchar()) != '\n' && c != EOF){
        //Clearing Stdin
    }
    return key;
}
void getString(char *buf)
{
    fgets(buf, SIZE, stdin);
    return;
}
void encrypt(char *buf, int shift)
{
    int i = 0;
    for(i = 0; i <= (strlen(buf)-1);++i)
    {
        buf[i] = buf[i] + shift;
    }
    return;
}
void decrypt(char *buf, int shift)
{
    int i = 0;
    for(i = 0; i <= (strlen(buf)-1); ++i)
    {
        buf[i] = buf[i] - shift;
    }
    return;
}
int main()
{
    char userStr[SIZE];
    int shift = 3;

    while(1){
        int number = getUserChoice();
        if(number == 1)
        {
            shift = getShift();

        }
        if (number == 2)
        {
            getString(userStr);
            printf("Message: %s\n", userStr);
            encrypt(userStr, shift);
            printf("Shifted Message: %s\n", userStr);
        }
        if (number == 3)
        {
            getString(userStr);
            printf("Shifted Message: %s\n", userStr);
            decrypt(userStr, shift);
            printf("Message: %s\n", userStr);
        }
        if (number == 0){
            break;
        }
    }
    return 0;
}

Answer 2

The are two things in the code that cause it to ignore the user input.

First, the getUserChoice function returns a value, but main doesn't store or use the return value. The function call should look like this:

number = getUserChoice();

Second, a single equal sign if(number = 1) acts as an assignment. It sets number to the value 1. To compare number with 1, use the equality operator, e.g.

if ( number == 1 )

Note that any decent compiler will generate a warning when you mistakenly put an assignment inside an if statement. You should make sure that you enable all warnings (use -Wall with gcc or clang, /W4 with microsoft), and fix all of the warnings. If you ignore compiler warnings, you'll get nowhere fast.

There are other problems in your code, but those are the two problems that are causing the user input to be ignored.