Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is a tiny learning program that lead to an interesting question: how can I best/most elegantly handle user entered numbers? This method works, fails cleanly, and reads well. It doesn't apply to types other than build in ones (as far as I know), but for floats and integers it's a nice feature. But it doesn't scale well, and seems cumbersome. Thoughts?

Read two numbers (a,b) then increment (a) by one if ( a > b ) and (a > 0) otherwise decrement (a) by one. Print (a).

Sample Input:
27
36

Sample Output:
26

#include <iostream>
#include <stdexcept>

int main() {
    int a, b;

    try {
        std::cin >> a;
        if (std::cin.fail()) throw std::runtime_error("Input is not an integer\n");
        std::cin >> b;
        if (std::cin.fail()) throw std::runtime_error("Input is not an integer\n");

    } catch (const std::runtime_error& e) {
        std::cout << e.what();
        return 1;
    }

    if (a > b && a > 0) {
        ++a;
    } else {
        --a;
    }

    std::cout << a;

    return 0;
} 
share|improve this question
1  
What do you mean by "doesn't scale well"? –  ruds Jul 18 at 23:21

3 Answers 3

up vote 3 down vote accepted

If you are already using exceptions then why not use the ones provided by std::basic_istream (http://en.cppreference.com/w/cpp/io/basic_ios/exceptions):

std::cin.exceptions(std::ios_base::failbit);
try {
    std::cin >> a;
    std::cin >> b;
catch(const std::ios_base::failure &failure) {
    //...
}

If you are using this, be sure to save (oldExceptionBits = std::cin.exceptions()) and later restore (std::cin.exceptions(oldExceptionBits)) the exception state of cin as other code might not be prepared for operations on cin to throw.

share|improve this answer
    
This is a good point. This addresses my "scaling" concern as a separate check for failure isn't required for each prompt and means I'm not replicating code already in iostream. –  Derek Jul 22 at 10:24

I question your use of exceptions to catch the invalid input. Exceptions are supposed to be used for exceptional situations. Validating user input is part of the normal functionality of the program, not an exceptional situation. It's on the lazy side to use exceptions here; you're basically treating it as a goto.

When you do use exceptions, you should probably create a new class that's derived from one of the standard exception classes, and throw/catch that instead of the one from <stdexcept>.

I also think that when you fail, you should inform the user of that, reset the input stream, and give them another chance, rather than just exiting.

Also, sure this is just a toy, but in a real program you should separate the business logic of your program from the input and output. You might want to plug the business logic into adapters that get input from a different place and/or send the output to a different place.

share|improve this answer
    
In this case it might be true (as the newly thrown exceptions are raised at the same level where they are caught) that the exceptions are a disguised goto. However, I would argue that exceptions are a good fit to indicate parsing errors. They have a clear edge over errors that are returned from the function or must be requested via special error functions (like failbit()) because the latter can easily be ignored/forgotten. –  Nobody Jul 19 at 10:41

One way to validate the input and keep the code robust, is to read the input as a string and use a helper function to try and parse the string to a number:

bool IntTryParse(string input, int& output)
{
    output = -1;
    for (char c : input)
        if (!isdigit(c))
            return false;
    sscanf(input.c_str(),"%d", &output);
    return true;
}

To get the input something like this would work:

string temp = "";
int num1 = 0;
bool done = false;
while (!done)
{
    cout << "Enter a number.\n";
    cin >> temp;
    if (IntTryParse(temp, num1))
        break;
    cout << "Please enter a valid number\n";
}

When the while loop exits num1 will hold a valid number. This only checks for positive integers, but the concept is simple enough to add any other validation you want. This doesn't check for out of range for an int, either.

share|improve this answer
1  
Using std::stoi is another alternative here. It's exceptions address invalid inputs and out of range errors. It does allow for trailing non digit characters though interestingly. en.cppreference.com/w/cpp/string/basic_string/stol –  Derek Jul 22 at 10:26

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.