Data validation problem

Hello,

My father and I have made this calculator. It works, but I want to include data validation so that if the user does not enter 'y' to continue or 'q' to quit, it will tell the user that he has made an error and prompt the user to try again.

I've tried using a "do...while" and a "while" but cannot get it to work.

Also, ignore my fathers abuse of "else". He likes to use it because it's easier for him to read because he was a COBOL programmer for years.

Any help would be appreciated.

Code:

#include <stdio.h>
#include <stdlib.h>
int main(void)
{
    double num1 = 0;
    double num2 = 0;
    char operand[1];
    char exitsw = ' ';
  do
  {
    printf ("\nKey 1st Number, +, -, *, or /, 2nd Number, and Press Enter.\n\n");
    scanf("%lf %c %lf", &num1, operand, &num2);
    if (operand[0] == '+')
       printf ("Sum of two numbers is %f \n", num1+num2);
    else if (operand[0] == '-')
       printf ("Difference between two numbers is %f \n", num1-num2);
    else if (operand[0] == '*')
       printf ("Product of two numbers is %f \n", num1*num2);
    else if (operand[0] == '/' && num2 == 0)
       printf ("Resubmit: Zero Divisor Is Not Valid \n");
    else if (operand[0] == '/')
       printf ("Division of two numbers is %f \n", num1/num2);
    else
       printf ("Resubmit: Invalid Operand \n");
    fflush(stdin);
    
    printf ("\nWould you like to make another calculation?\n");
    printf ("\nTo make another calculation, enter Y.\n");
    printf ("\nTo Quit, enter q.\n");
    scanf("%c", &exitsw);
    }while (exitsw == 'y' || exitsw == 'Y');
    
return 0;


}

First try this

instead of

Code:

scanf("%c", &exitsw);

use

Code:

scanf(" %c", &exitsw);

The space before the %c tells the scanf function to skip over whitespace like the newline and spaces.

Next read the FAQ > Why fflush(stdin) is wrong - Cprogramming.com

Tim S.

So after the user enters exitsw,

exitsw=tolower(exitsw)
if(exitsw != 'y' && exitsw != 'q') exitsw='y' and print your message to enter y or q, and let the loop iterate around.


I would be sure to add one space before the % in your scanf(). It can be tripped up, otherwise. Also, fflush(stdin) doesn't work. Only works on output files, not input.

First there are a few issues with the code here. Number 1, why do you declare operand as a char array instead of a single char?
You then use it invalidly in the scanf call.

Change

Code:

           char operand[1];
...
...
           scanf("%lf %c %lf", &num1, operand, &num2);

to

Code:

           char operand;
...
...
           scanf("%lf %c %lf", &num1, &operand, &num2);

And then you will need to change all your conditions to be just if (operand == '*') etc.. instead of operand[0]. Not sure what your logic was there..

Second issue is the use of calling fflush() on stdin this is a BIG no-no as it causes undefined behavior. Google it if you don't know what it is.

Code:

    /* Waiting to cause Segfault/random error (undefined behavior) */
    fflush(stdin);

What you need to do is use something better then scanf() but as a quick fix you can put a space in front of your calls. So you can do:

Code:

   scanf(" %c", &exitsw);

This is a little trick that will make scanf() ignore leading whitespace (stray newline characters for example) in the input stream. You should
really be using a better form of input capture some combination of fgets()/sscanf or something of that nature.

Finally, as far as your question on input validation--You can fix this easily by inverting the logic on your condition:

Code:

   }while (exitsw != 'q' && exitsw != 'Q');

Triple redundant posting all in the same minute...damn editor kept bombing out on me or would've posted earlier *rolls eyes*...

Well, for once, I wasn't the slowest fingers in the bunch! We'll have to up our keyboarding rate to catch "fast fingers" Tim, though.

Originally Posted by Adak

Well, for once, I wasn't the slowest fingers in the bunch! We'll have to up our keyboarding rate to catch "fast fingers" Tim, though.

I am likely one of the slowest typist on this forum. It one of the things I need to improve on.

Tim S.

Edit: sorry didn't expect 10 people to beat me to it -.-

Code:

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

int main(void)
{
    double num1 = 0;
    double num2 = 0;
    // Operand is a character not an array of characters.
    char operand;
    // Declare a buffer for fgets (I chose 5, you should make it as big as you need)
    char buffer[5];
    do
    {
        printf ("\nKey 1st Number, +, -, *, or /, 2nd Number, and Press Enter.\n\n");
        // Using fgets read in the entire input line
        fgets(buffer,5,stdin);
        // Convert the string into the numbers and character
        sscanf(buffer,"%lf %c %lf", &num1, &operand, &num2);
        if (operand == '+')
           printf ("Sum of two numbers is %f \n", num1+num2);
        else if (operand == '-')
           printf ("Difference between two numbers is %f \n", num1-num2);
        else if (operand == '*')
           printf ("Product of two numbers is %f \n", num1*num2);
        else if (operand == '/' && num2 == 0)
           printf ("Resubmit: Zero Divisor Is Not Valid \n");
        else if (operand == '/')
           printf ("Division of two numbers is %f \n", num1/num2);
        else
           printf ("Resubmit: Invalid Operand \n");

        printf ("\nWould you like to make another calculation?\n");
        printf ("\nTo make another calculation, enter Y.\n");
        printf ("\nTo Quit, enter q.\n");
        // Use fgets to store the users "Quit" choice.
        fgets(buffer,5,stdin);
        // Compare the first character of the string to get the choice,
        // Really could do more validation using strncmp but this suffices
    }while(buffer[0] != 'Q' && buffer[0] != 'q');

    return 0;
}
}

I HIGHLY recommend avoiding scanf, by doing so you avoid all the issues of trailing newlines, by using fgets and sscanf you save yourself a lot of misery.

Originally Posted by nonpuz

First there are a few issues with the code here. Number 1, why do you declare operand as a char array instead of a single char?
You then use it invalidly in the scanf call.

Change

Code:

           char operand[1];
...
...
           scanf("%lf %c %lf", &num1, operand, &num2);

to

Code:

           char operand;
...
...
           scanf("%lf %c %lf", &num1, &operand, &num2);

And then you will need to change all your conditions to be just if (operand == '*') etc.. instead of operand[0]. Not sure what your logic was there..

Second issue is the use of calling fflush() on stdin this is a BIG no-no as it causes undefined behavior. Google it if you don't know what it is.

Code:

    /* Waiting to cause Segfault/random error (undefined behavior) */
    fflush(stdin);

What you need to do is use something better then scanf() but as a quick fix you can put a space in front of your calls. So you can do:

Code:

   scanf(" %c", &exitsw);

This is a little trick that will make scanf() ignore leading whitespace (stray newline characters for example) in the input stream. You should
really be using a better form of input capture some combination of fgets()/sscanf or something of that nature.

Finally, as far as your question on input validation--You can fix this easily by inverting the logic on your condition:

Code:

   }while (exitsw != 'q' && exitsw != 'Q');

Not sure why my father wanted to use an array.

I like your other suggestions. One problem though...

Lets say the user wants to quit. He goes to enter 'q', but accidentally enters 'w'.

The program starts over rather than telling him that he has made a mistake.

I want it to tell him that he has made an error, then prompt him to try again. I want it to repeat the loop no matter how many times the user fails to enter either 'y' or 'q'.

Originally Posted by Tomwa

Edit: sorry didn't expect 10 people to beat me to it -.-

Your solution has the same issue. If the user accidentally enters 'w', it does not tell him that he has made an error.

Then you sound like you want a

while(not good input and not quit input) print a message to have them re-enter their choice, and get that new input inside the while loop.

Code:

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

int main(void)
{
    double num1 = 0;
    double num2 = 0;
    // Operand is a character not an array of characters.
    char operand;
    // Declare a buffer for fgets (I chose 5, you should make it as big as you need)
    char buffer[5];
    do
    {
        printf ("\nKey 1st Number, +, -, *, or /, 2nd Number, and Press Enter.\n\n");
        // Using fgets read in the entire input line
        fgets(buffer,5,stdin);
        // Convert the string into the numbers and character
        sscanf(buffer,"%lf %c %lf", &num1, &operand, &num2);
        if (operand == '+')
           printf ("Sum of two numbers is %f \n", num1+num2);
        else if (operand == '-')
           printf ("Difference between two numbers is %f \n", num1-num2);
        else if (operand == '*')
           printf ("Product of two numbers is %f \n", num1*num2);
        else if (operand == '/' && num2 == 0)
           printf ("Resubmit: Zero Divisor Is Not Valid \n");
        else if (operand == '/')
           printf ("Division of two numbers is %f \n", num1/num2);
        else
           printf ("Resubmit: Invalid Operand \n");

           printf ("\nWould you like to make another calculation?\n");
           printf ("\nTo make another calculation, enter Y.\n");
           printf ("\nTo Quit, enter q.\n");
           fgets(buffer,5,stdin);
           // Convert character to upper so we don't have to check for upper and lower cases
           buffer[0] = toupper(buffer[0]);

           // Ensure input is a valid choice, if not inform and re-prompt the user
           while(buffer[0] != 'Q' && buffer[0] != 'Y')
           {
               printf("Invalid input, try again!");
               // Use fgets to store the users "Quit" choice.
               fgets(buffer,5,stdin);
               buffer[0] = toupper(buffer[0]);
           }

        // Compare the first character of the string to get the choice,
        // Really could do more validation using strncmp but this suffices
    }while(buffer[0] != 'Q')

    return 0;
}

Does this meet your specifications?

Originally Posted by Tomwa

Does this meet your specifications?

Eh, I suggest that you avoid doing other people's entire programs for them, at least not until they have arrived at a solution and you're offering a better one. Also, note that your indentation is off: the code after the "invalid operand" message is printed should not be indented at the same level since that would be misleading.

Originally Posted by Tomwa

Code:

...
           fgets(buffer,5,stdin);
           // Convert character to upper so we don't have to check for upper and lower cases
           buffer[0] = toupper(buffer[0]);
 
           // Ensure input is a valid choice, if not inform and re-prompt the user
           while(buffer[0] != 'Q' && buffer[0] != 'Y')
           {
               printf("Invalid input, try again!");
               // Use fgets to store the users "Quit" choice.
               fgets(buffer,5,stdin);
....

Does this meet your specifications?

What would happen if I enter the string:
1234q

How would you fix this problem? (Extra credit)

Originally Posted by laserlight

Eh, I suggest that you avoid doing other people's entire programs for them, at least not until they have arrived at a solution and you're offering a better one.

99% of that code is his, Technically he had a working solution (Did the math and received input), and I improved on it by adding sscanf/fgets, the use of toupper to remove the need to compare to lower and uppercase versions of characters, and a while loop to validate the yes or no (Which he already basically had).

Originally Posted by laserlight

Also, note that your indentation is off: the code after the "invalid operand" message is printed should not be indented at the same level since that would be misleading.

I didn't realize that, when I moved the code by highlighting it and dragging it I failed to notice the improper indentation, that was a mistake on my part, it should be tab-shifted to the left.