This is a drill from the Programming Principles and Practice Using C++. I managed to get it to work but somehow it doesn't feel right to me. I 'd appreciate any pointers on how to simplify/improve the solution.

Especially when it comes to best check if '|' was entered. I feel that creating two extra string variables and then converting them to int using std::stoi is very inefficient. I am new to C++ so please excuse the ignorance. Appreciate any feedback.

Question

Write a program that in a while loop reads 2 integers and prints them. Carry on until the user enters '|'.

#include<iostream>
#include<string>

int main(){
    int a, b;
    std::string _a, _b;
    while (true){
        std::cout << "Enter two numbers." << std::endl;
        std::cin >> _a >> _b;
        if (_a == "|" || _b == "|"){break;};
        a = std::stoi(_a);
        b = std::stoi(_b);
        std::cout << "You entered " << a << " and " << b << std::endl;
    }
    return 0;
}
up vote 5 down vote accepted
        std::cout << "Enter two numbers." << std::endl;

You are looking to accept integers, not just numbers (i.e. \$1.0, 1e0, 0b1, 0x1\$).

Avoid std::endl. std::endl inserts a newline character into an output sequence and flushes it as if by calling stream.put(stream.widen('\n')) followed by stream.flush(). If you just want to insert a newline character, then stream '\n'.

std::cout << "Enter two integers:\n"; // can append it on C strings

        std::cin >> _a >> _b;

Think about how you want to handle inputs. You are expecting 2 integers, but what if they pass you one integer? No integers (or send end-of-file)? For those cases, you should check if the value was read successfull (see 10.10 in your book).

When a value cannot be extracted from the stream, the streamable types in C++ will typically set std::ios::failbit. When std::string is the type being extracted, a failure to extract leaves the data in the string unchanged. For other types, like the numeric types, the extraction operation will assign 0 on failure.

If the stream enters a fail state, you won't be able to extract from it until you clear() the error flag. Not clearing the error flag would result in an infinite loop once the stream entered the error state.

        std::cin.clear();

What if the user enters more than 2 integers? Should you keep reading from the line? Doing this, you'll need to make sure both values are extracted, even if one failed. Could you ignore() the remaining input until end of line? You were only expecting two integers, so this is an option.

        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

        a = std::stoi(_a);
        b = std::stoi(_b);
        std::cout << "You entered " << a << " and " << b << std::endl;

What happens if the extraction from std::cin fails? Either _a, _b, or both could be empty and std::stoi will throw std::invalid_argument when an empty string is encountered.

std::stoi will truncate at the decimal separator and return the number to that point. If you want to validate that the string represents an integer, you need to make sure std::stoi reads the full buffer.

        std::size_t last_pos_read;
        a = std::stoi(_a, std::address_of(last_pos_read));
        if (last_pos_read != _a.length()) {
            throw std::invalid_argument("Not an integer.");
        }

Inexperience aside, the big lesson you should take from this review is to test! Test inputs of type integer (negatives, zero, positives, 16/32/64bit values), floating point (representable/nonrepresentable values), string/containers (empty/nonempty, small/large sizes). Test failures at different times, like an immediate failure or a failure after a successful loop.

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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