6
\$\begingroup\$

I finally worked out all of the bugs in this bit of code. I'm pretty proud of it and I wanted to give it to all of you so that you could have it if you wanted it for any console programs that you make.

At the same time, this is ridiculous. C++ is double my age and there should seriously be a better way of getting valid, error free, inputs from the user. If anyone knows, please tell me.

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>

void gValid_Input(std::string& var, std::string question) {
    using namespace std;

    do {
        cin.clear();
        cin.sync();
        cout << question;

        // Code will not work as intended on Linux without the following
        // but will work on windows (tested with g++)
        // this must also be done after clearing, syncing cin
        // and asking the question string otherwise the code will loop forever
        if (cin.peek() == '\n'){
            cin.ignore(1,'\n');
        }
    } while (!(getline(cin, var)));
}

template <typename t>
void gValid_Input(t& var, std::string question) {
    using namespace std;

    do {
        cin.clear();
        cin.sync();
        cout << question;
    } while (!(cin >> var));
}

void gValid_Option(char& response, std::vector<char> valid_Responses = {'y','n'}){
    using namespace std;
    const char diff = 'a' - 'A'; 

    do{
        cin.clear();
        cin.sync();
        cin >> response;

        // Comment the following out if you care about getting an upper case answer
        if (response >= 'A' && response <= 'Z'){
            response += diff;
        }

    } while (find(valid_Responses.begin(), valid_Responses.end(), response) == valid_Responses.end());
}

void gValid_Option(char& response, std::string question, std::vector<char> valid_Responses = {'y','n'}){
    using namespace std;
    const char diff = 'a' - 'A';

    do{
        cin.clear();
        cin.sync();
        cout << question;
        cin >> response;

        // Comment the following out if you care about getting an upper case answer
        if (response >= 'A' && response <= 'Z'){
            response += diff;
        }

    } while (find(valid_Responses.begin(), valid_Responses.end(), response) == valid_Responses.end());
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I see some things that may help you improve your program.

Use const references where practical

The code currently declares its first function like so:

void gValid_Input(std::string& var, std::string question);

However the passed question is not and should not be altered by this code, so better would be to pass a const std::string& question or std::string_view question if you have a C++17 compiler.

Use return values to return values

Each of the functions is defined as returning void but each is actually returning a single value which is the response. So for the first function, including the advise listed above, one could instead make the signature of the first function this:

std::string gValid_Input(const std::string& question);

Fix the bug

When the user inputs the end of file marker (Ctrl+D in Linux), the prompt just repeats endlessly in a loop.

Consolidate the code

All four of the functions could be represented with a single templated function:

template <typename T>
T gValid_Input(const std::string& prompt, bool (*isValid)(const T&) = [](const T&){ return true; }) {
    T var;
    while (std::cout << prompt && !((std::cin >> var) && isValid(var))) {
        if (std::cin) {
            std::string line;
            std::getline(std::cin, line);
            std::cout << var << line << " is not a valid answer\n";
        } else {
            std::cin.clear();
            std::string line;
            std::getline(std::cin, line);
            std::cout << line << " is not a valid answer\n";
        }
    }
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    return var;
}

Here's are some example of its use:

#include <complex>
#include <array>

int main() {
    auto answer{gValid_Input<std::string>("What is your name? ")};
    std::cout << "Your answer: \"" << answer << "\"\n";
    auto age{gValid_Input<int>("What is your age? ")};
    std::cout << "Your answer: \"" << age << "\"\n";
    auto cpx{gValid_Input<std::complex<double>>("What is your favorite complex number? ")};
    std::cout << "Your answer: \"" << cpx << "\"\n";
    auto weekday{gValid_Input<std::string>("What is your favorite day of the week? ", 
        [](const std::string &day){ const std::array<std::string, 7> weekdays{
            "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday" 
            };
            return std::find(weekdays.begin(), weekdays.end(), day) != std::end(weekdays);
        }
    )};
    std::cout << "Your answer: \"" << weekday << "\"\n";
    auto fun{gValid_Input<char>("Are you having fun? ", 
        [](const char &ch){ return ch=='y' || ch=='n'; }
    )};
    std::cout << "Your answer: \"" << fun << "\"\n";
}

I'm using a lambda function as the validator, but it could also be any function with the appropriate signature.

Reconsider the interface

It would be nice to be able to use streams other than std::cin and std::cout. I'd recommend passing the input and output streams as parameters.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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