0

I am trying to make the program return an error if the user input characters/strings to num1, num2 and then take a right input instead

I searched the internet and found the solution with cin.fail() but it works only for the first number or doesn't work at all

I was advised to do it by figuring out the type of variable and then compare but I cant figure it out

How can I make validation in easiest simplest way possible without functions but loops?

#include <iostream>
#include <fstream>
using namespace std;

int main () {

    int num1, num2; // two integers
    int choice; // chosen integers
    int ch1 = 0, ch2 = 0,ch3 = 0,ch4 = 0,ch5 = 0; // to keep track of chosen functions
    
    //const for choice of menu
    const int addition = 1,
              substruction = 2,
              multiplication = 3,
              division = 4, 
              modulo = 5,
              numberChange = 6;


    
    //intro
    cout << "This program has been made for arithmetic operations.\n";
    cout << "Please choose two  integers for arithmetic operations\n";
    cin >> num1 >> num2;


    do {
        cout << "Choose number of the operation you want to do\n";
        cout << "1. addition\n";
        cout << "2. subtraction\n";
        cout << "3. multiplication\n";
        cout << "4. division\n";
        cout << "5. modulo\n";
        cout << "6. CHANGE OF NUMBERS\n";
        cout << "To exit menu enter -1!\n";
        cin >> choice;

       

        switch (choice) {
            case 1:
            cout << "Result: " <<num1  << " + " << num2 << " = "<< num1 + num2 << endl;
            ch1++;
            break;
            case 2:
            cout << "Result: " <<num1  << " - " << num2 << " = "<< num1 - num2 << endl;
            ch2++;
            break;
            case 3: 
            cout << "Result: " <<num1  << " * " << num2 << " = "<<num1 * num2 << endl;
            ch3++;
            break;
            case 4:
            cout << "Result: " <<num1  << " / " << num2 << " = "<<num1 / num2 << endl;
            ch4++;
            break;
            case 5:
            cout << "Result: " <<num1  << " % " << num2 << " = "<<num1 % num2 << endl;
            ch5++;
            break;
            case 6:
            cout << "Okay, you can choose different numbers" << endl;
            cin >> num1 >> num2;
            break;
            case -1:

            ofstream myfile ("/Users/margaritakholostova/Desktop/us.txt");
            if (myfile.is_open())
             {
              myfile << "Addition was selected " << ch1 << " times\n";
              myfile << "Subtraction was selected " << ch2 << " times\n";
              myfile << "Multiplication was selected " << ch3 << " times\n";
              myfile << "Division was selected " << ch4 << " times\n";
              myfile << "Modulo was selected " << ch5 << " times\n";
              myfile.close();
            }
            else cout << "Unable to open file";
            return 0; 
            break;
        }

        //return error if input is not integer
        if (cin.fail()) {
            cout << "Error! Please put only integers!" << endl;
            cin.clear();
            cin.ignore(256, '\n');
            cin >> choice;
        }


        // validate the input for right number

        while (choice < addition || choice > numberChange )
        {
            if (choice != -1) {
                cout << "Please choose valid option out of menu (1-5)\n";
                cin >> choice;
            }
            
        }

    } while (choice != -1);        
}
M.khm
  • 35
  • 5
  • What's wrong with getting and validating the inputs one at a time? You don't have to tell the user that the first read failed until after getting the second if you want. – user4581301 Apr 13 '22 at 00:26
  • Do you want both numbers to be entered by the user on the same line or in different lines? Or does it not matter to you? – Andreas Wenzel Apr 13 '22 at 00:32
  • Doesn’t matter the same line or not; – M.khm Apr 13 '22 at 00:34
  • @user4581301 how do I check two numbers at once? – M.khm Apr 13 '22 at 00:35
  • Note that using `cin >> num1` will accept input such as `6abc27` as valid input for the number `6`. Is this what you want? If you want such input to be rejected instead, then you should not be using `std::cin::operator>>`. You may want to take a look at my function `get_int_from_user` that I wrote in the code snippet from [this answer of mine to another question](https://stackoverflow.com/a/70818264/12149471). That function will perform very strict input validation for a single number, and reprompt the user for input if necessary. – Andreas Wenzel Apr 13 '22 at 00:36
  • The function `get_int_from_user` that I mentioned in my comment above will only read a single number of input from the user. In order to use it for inputting two numbers, you can simply call the function twice. – Andreas Wenzel Apr 13 '22 at 00:40
  • 1
    Instead of using `cin >> num1 >> num2;`, read user input as a `std::string` (e.g. `std::getline(std::cin, some_string)`) and then parse the string. That gives flexibility to check input any way you wish (e.g. discard or diagnose bad input) or to accept input from one or multiple lines (e.g. by calling `getline()` twice). Whereas `cin >> num1` places `std::cin` in an error state on bad input (so your code needs to clear that before attempting to read more input), can register some input as good even if bad (e.g. input `6xyz2` will read the `6` but result in error on the `x`). – Peter Apr 13 '22 at 00:47
  • Don't waste your time trying to do two at a time. Get both at the same time, test them separately and return an error unless both pass. I do what Peter suggested above, read into a `string`. To convert the string into integer, I'll use `std::stoi` or `strtol` because they can catch failed conversions, out of range numbers, and conversions that ended early and didn't use the whole `string`. Write a function that converts and validates one number and call it twice. Problem solved. – user4581301 Apr 13 '22 at 00:53

2 Answers2

1

You need to check the error state of std::cin before you act on the value returned by operator>>. Try something more like this instead:

#include <iostream>
#include <fstream>
#include <limits>
using namespace std;

int main () {

    int num1, num2; // two integers
    int choice; // chosen integers
    int ch1 = 0, ch2 = 0, ch3 = 0, ch4 = 0, ch5 = 0; // to keep track of chosen functions
    
    //const for choice of menu
    const int addition = 1,
              substruction = 2,
              multiplication = 3,
              division = 4, 
              modulo = 5,
              numberChange = 6;

    //intro
    cout << "This program has been made for arithmetic operations.\n";
    cout << "Please choose two integers for arithmetic operations\n";

    //return error if input is not integers
    while (!(cin >> num1 >> num2)) {
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
        cout << "Error! Please enter only integers!\n";
    }

    do {
        cout << "Choose number of the operation you want to do\n";
        cout << "1. addition\n";
        cout << "2. subtraction\n";
        cout << "3. multiplication\n";
        cout << "4. division\n";
        cout << "5. modulo\n";
        cout << "6. CHANGE OF NUMBERS\n";
        cout << "To exit menu enter -1!\n";

        //return error if input is not integer
        while (!(cin >> choice)) {
            cin.clear();
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            cout << "Error! Please enter only integers!\n";
        }

        // validate the input for right number
        switch (choice) {
            case 1:
                cout << "Result: " << num1 << " + " << num2 << " = " << num1 + num2 << '\n';
                ch1++;
                break;
            case 2:
                cout << "Result: " << num1 << " - " << num2 << " = " << num1 - num2 << '\n';
                ch2++;
                break;
            case 3: 
                cout << "Result: " << num1 << " * " << num2 << " = " << num1 * num2 << '\n';
                ch3++;
                break;
            case 4:
                cout << "Result: " << num1 << " / " << num2 << " = " << num1 / num2 << '\n';
                ch4++;
                break;
            case 5:
                cout << "Result: " << num1 << " % " << num2 << " = " << num1 % num2 << '\n';
                ch5++;
                break;
            case 6:
                cout << "Okay, you can choose different numbers\n";
                while (!(cin >> num1 >> num2)) {
                    cin.clear();
                    cin.ignore(numeric_limits<streamsize>::max(), '\n');
                    cout << "Error! Please enter only integers!\n";
                }
                break;
            case -1: {
                ofstream myfile ("/Users/margaritakholostova/Desktop/us.txt");
                if (myfile.is_open()) {
                    myfile << "Addition was selected " << ch1 << " times\n";
                    myfile << "Subtraction was selected " << ch2 << " times\n";
                    myfile << "Multiplication was selected " << ch3 << " times\n";
                    myfile << "Division was selected " << ch4 << " times\n";
                    myfile << "Modulo was selected " << ch5 << " times\n";
                    myfile.close();
                }
                else
                    cout << "Unable to open file\n";
                return 0; 
            }

            default:
                cout << "Please choose a valid option from the menu (1-5)\n";
                break;
        }
    }
    while (true);
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Now it works for validation, but now it skips two first inputs; So I input 4 numbers and it counts only last 2 – M.khm Apr 13 '22 at 23:48
  • "*now it skips two first inputs*" - [works fine for me](https://ideone.com/jCrhNX), no skips at all. – Remy Lebeau Apr 14 '22 at 00:05
1

The lines

if ( cin.fail() )
{
    cout << "Error! Please put only integers!" << endl;
    cin.clear();
    cin.ignore(256, '\n');
    cin >> choice;
}

will not work properly, because at the time that you perform the check for stream failure, you will have already used the result of

cin >> num1 >> num2;

and

cin >> choice;

inside the switch statement.

Therefore, it would make sense to perform such a check before the switch statement.

On the other hand, instead of writing code that performs such a check after every single input operation, it would probably be more meaningful to write a separate function get_int_from_user that reads one line of input and attempts to convert it to an integer. If this conversion fails, then this function should automatically reprompt the user for input, until the conversion succeeds. That way, all you have to do in your main code is to call the function get_int_from_user, and that function will handle most of the input validation for you. Since the function get_int_from_user is guaranteed to return a valid integer, the only input validation that you will have to perform in your main code is whether the integer is in the desired range. This will make your main code a lot easier to write.

Another issue is that it would make more sense to put the range check

while (choice < addition || choice > numberChange )
{
    if (choice != -1) {
        cout << "Please choose valid option out of menu (1-5)\n";
        cin >> choice;
    }

}

inside the switch statement. This can easily be done by using the default label, which will be executed if the value does not match any of the case labels.

In the following code snippet, I have provided my solution to the problem. Note however that this solution uses three goto labels. The goto statement should generally be used cautiously, as it is bad programming practice to make excessive use of goto. It is usually better to use a proper for, while or do loop instead. However, using goto to break out of nested loops is generally considered appropriate. In my solution, I needed two goto labels anyway, in order to break out of nested loops. In this case, I decided that introducing a third goto label and getting rid of a nested loop was appropriate, as it made the code clearer. However, since most other programmers will probably consider this decision of mine controversial, I have provided an alternative solution at the end of my answer, in which I have removed both goto labels from the function menu, so that only the function get_int_from_user uses a goto label, which it does strictly for breaking out of a nested loop.

Here is my solution which uses three goto labels:

#include <iostream>
#include <string>
#include <stdexcept>
#include <cctype>

int get_int_from_user( const std::string& prompt );

void menu()
{
    int num1, num2;
    int choice;
    int ch1 = 0, ch2 = 0, ch3 = 0, ch4 = 0, ch5 = 0;

    //intro
    std::cout << "This program has been made for arithmetic operations.\n";

new_numbers:
    std::cout << "Please choose two integers for arithmetic operations.\n";

    num1 = get_int_from_user( "Please enter the first number: " );
    num2 = get_int_from_user( "Please enter the second number: " );

menu:
    std::cout <<
        "\n"
        "Choose number of the operation you want to do\n"
        "1. addition\n"
        "2. subtraction\n"
        "3. multiplication\n"
        "4. division\n"
        "5. modulo\n"
        "6. CHANGE OF NUMBERS\n"
        "To exit menu enter -1!\n"
        "\n";

    choice = get_int_from_user( "Please enter your choice: " );

    switch ( choice )
    {
        case 1:
            std::cout << "\nResult: " <<num1  << " + " << num2 << " = " << num1 + num2 << "\n\n";
            ch1++;
            goto menu;

        case 2:
            std::cout << "\nResult: " <<num1  << " - " << num2 << " = "<< num1 - num2 << "\n\n";
            ch2++;
            goto menu;

        case 3: 
            std::cout << "\nResult: " <<num1  << " * " << num2 << " = "<<num1 * num2 << "\n\n";
            ch3++;
            goto menu;

        case 4:
            std::cout << "\nResult: " <<num1  << " / " << num2 << " = "<<num1 / num2 << "\n\n";
            ch4++;
            goto menu;

        case 5:
            std::cout << "\nResult: " <<num1  << " % " << num2 << " = "<<num1 % num2 << "\n\n";
            ch5++;
            goto menu;

        case 6:
            std::cout << "Okay, you can choose different numbers.\n" << '\n';
            goto new_numbers;

        case -1:
            break;

        default:
            std::cout << "Please choose valid option out of menu (1-5)\n";
            goto menu;
    }

    std::cout <<
        "\n" <<
        "Addition was selected " << ch1 << " times\n" <<
        "Subtraction was selected " << ch2 << " times\n" <<
        "Multiplication was selected " << ch3 << " times\n" <<
        "Division was selected " << ch4 << " times\n" <<
        "Modulo was selected " << ch5 << " times\n";
}

int main()
{
    try
    {
        menu();
    }
    catch ( std::runtime_error& e )
    {
        std::cout << "Runtime error: " << e.what() << '\n';
    }
}

int get_int_from_user( const std::string& prompt )
{
    std::string line;
    std::size_t pos;
    int i;

    //repeat forever, until an explicit return statement or an
    //exception is thrown
    for (;;) //equivalent to while(true)
    {
        //prompt user for input
        std::cout << prompt;

        //attempt to read one line of input from user
        if ( !std::getline( std::cin, line ) )
        {
            throw std::runtime_error( "unexpected input error!\n" );
        }

        //attempt to convert string to integer
        try
        {
            i = std::stoi( line, &pos );
        }
        catch ( std::invalid_argument& )
        {
            std::cout << "Unable to convert input to number, try again!\n";
            continue;
        }
        catch ( std::out_of_range& )
        {
            std::cout << "Out of range error, try again!\n";
            continue;
        }

        //The remainder of the line is only allowed to contain
        //whitespace characters. The presence of any other
        //characters should cause the entire input to get rejected.
        //That way, input such as "6sdfj23jlj" will get rejected,
        //but input with trailing whitespace will still be accepted
        //(just like input with leading whitespace is accepted by
        //the function std::stoi).
        for ( ; pos < line.length(); pos++ )
        {
            if ( !std::isspace( static_cast<unsigned char>(line[pos]) ) )
            {
                std::cout << "Invalid character found, try again!\n";

                //we cannot use "continue" here, because that would
                //continue to the next iteration of the innermost
                //loop, but we want to continue to the next iteration
                //of the outer loop
                goto continue_outer_loop;
            }
        }

        //input is valid
        return i;

    continue_outer_loop:
        continue;
    }
}

This program has the following behavior:

This program has been made for arithmetic operations.
Please choose two integers for arithmetic operations.
Please enter the first number: 3
Please enter the second number: 4

Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 1

Result: 3 + 4 = 7


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 2

Result: 3 - 4 = -1


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 3

Result: 3 * 4 = 12


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 4

Result: 3 / 4 = 0


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 5

Result: 3 % 4 = 3


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 6
Okay, you can choose different numbers.

Please choose two integers for arithmetic operations.
Please enter the first number: 5 
Please enter the second number: 7

Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 1

Result: 5 + 7 = 12


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 2

Result: 5 - 7 = -2


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 3

Result: 5 * 7 = 35


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 4

Result: 5 / 7 = 0


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: 5

Result: 5 % 7 = 5


Choose number of the operation you want to do
1. addition
2. subtraction
3. multiplication
4. division
5. modulo
6. CHANGE OF NUMBERS
To exit menu enter -1!

Please enter your choice: -1

Addition was selected 2 times
Subtraction was selected 2 times
Multiplication was selected 2 times
Division was selected 2 times
Modulo was selected 2 times

Due to using the function get_int_from_user, the program also has extensive input validation:

This program has been made for arithmetic operations.
Please choose two integers for arithmetic operations.
Please enter the first number: abc
Unable to convert input to number, try again!
Please enter the first number: 4abc
Invalid character found, try again!
Please enter the first number: 4
Please enter the second number: 

Here is the announced alternative version of the function menu, in which both goto labels are removed and replaced with a loop and an additional function:

void input_two_numbers( int &i1, int &i2 )
{
    i1 = get_int_from_user( "Please enter the first number: " );
    i2 = get_int_from_user( "Please enter the second number: " );
}

void menu()
{
    int num1, num2;
    int choice;
    int ch1 = 0, ch2 = 0, ch3 = 0, ch4 = 0, ch5 = 0;

    //intro
    std::cout << "This program has been made for arithmetic operations.\n";

    std::cout << "Please choose two integers for arithmetic operations.\n";

    input_two_numbers( num1, num2 );

    for (;;) //infinite loop
    {
        std::cout <<
            "\n"
            "Choose number of the operation you want to do\n"
            "1. addition\n"
            "2. subtraction\n"
            "3. multiplication\n"
            "4. division\n"
            "5. modulo\n"
            "6. CHANGE OF NUMBERS\n"
            "To exit menu enter -1!\n"
            "\n";

        choice = get_int_from_user( "Please enter your choice: " );

        switch ( choice )
        {
            case 1:
                std::cout << "\nResult: " <<num1  << " + " << num2 << " = " << num1 + num2 << "\n\n";
                ch1++;
                continue;

            case 2:
                std::cout << "\nResult: " <<num1  << " - " << num2 << " = "<< num1 - num2 << "\n\n";
                ch2++;
                continue;

            case 3: 
                std::cout << "\nResult: " <<num1  << " * " << num2 << " = "<<num1 * num2 << "\n\n";
                ch3++;
                continue;

            case 4:
                std::cout << "\nResult: " <<num1  << " / " << num2 << " = "<<num1 / num2 << "\n\n";
                ch4++;
                continue;

            case 5:
                std::cout << "\nResult: " <<num1  << " % " << num2 << " = "<<num1 % num2 << "\n\n";
                ch5++;
                continue;

            case 6:
                std::cout << "Okay, you can choose different numbers.\n" << '\n';
                input_two_numbers( num1, num2 );
                continue;

            case -1:
                break;

            default:
                std::cout << "Please choose valid option out of menu (1-5)\n";
                continue;
        }

        //break out of infinite loop
        break;
    }

    std::cout <<
         "\n" <<
         "Addition was selected " << ch1 << " times\n" <<
         "Subtraction was selected " << ch2 << " times\n" <<
         "Multiplication was selected " << ch3 << " times\n" <<
         "Division was selected " << ch4 << " times\n" <<
         "Modulo was selected " << ch5 << " times\n";
}

Personally, I prefer the version which uses three goto labels. In my opinion, having to create an additional function and also having to duplicate the call to this function makes the code harder to understand than simply using goto. But I am sure that many other programers will disagree with me, because using goto is highly controversial.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39