Simplifying if else user input validation

Question

This gets the job done for what I need it to do, but I am wondering if there is an easier/more efficient way of accomplishing the same thing. The user inputs two numbers and they need to be between 0 and 50, if it doesnt fall within the required range it ends the prog

cout << "Enter the pixel coordinate (x, y): ";
cin >> usrInput1 >> userInput2;
if  (usrInput1 > 50)
{
    cout << "ERROR! 1" << endl;
    return 0;
}
else if (usrInput1 < 0)
{
    cout << "ERROR! 2" << endl;         
    return 0;
}
else if (usrInput2 > 50)
{
    cout << "ERROR! 3" << endl;
    return 0;
}
else if (usrInput2 < 0)
{
    cout << "ERROR! 4" << endl;
    return 0;
}
else
{
    cout << "Success" << endl;
    xvar = usrInput1 + usrInput2;
}

I was trying to do something like

if(! 0 > userInput1 || userInput2 > 99)

but obviously that didn't work out..

Thanks for any assistance

If you need all the unique error messages, then there is no alternative. Otherwise you can combine them with || like in sehe's answer. — Oscar Korz
– Oscar Korz, Commented Oct 11, 2011 at 23:20

sehe · Accepted Answer · 2011-10-11 23:26:33Z

4

cout << "Enter the pixel coordinate (x, y): ";
cin >> usrInput1 >> userInput2;
if  ( (usrInput1 > 50) || (usrInput1 < 0)  ||
      (usrInput2 > 50) || (usrInput2 < 0) )
{
    cout << "ERROR!" << endl;
    return 0;
}
cout << "Success" << endl;
xvar = usrInput1 + usrInput2;

You could actually combine it further if you really wanted:

if  ((std::max(usrInput1,usrInput2) > 50) 
   || std::min(usrInput1,usrInput2) < 0))
{ 
     ...

in which case I would rather have a helper function

bool isValid(int i) { return (i>=0) && (i<=50); }

// ...
if (isValid(usrInput1) && isValid(usrInput2))
    ...

Edit Consider checking the input operations - this was missing in the OP:

if (!(cin >> usrInput1 >> userInput2))
{
     std::cerr << "input error" << std::endl;
}
if  ( (usrInput1 > 50) || (usrInput1 < 0)  ||
      (usrInput2 > 50) || (usrInput2 < 0) )
{
     std::cerr << "value out of range" << std::endl;
}

edited Oct 11, 2011 at 23:26

answered Oct 11, 2011 at 23:19

sehe

401k49 gold badges475 silver badges674 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Kerrek SB Over a year ago

The input operations should really be part of the conditional as well!

sehe Over a year ago

@KerrekSB; nice addition. I was however sticking to the OP (how to do the same more elegantly). I'll add it in for fun.

Extinct23 Over a year ago

@KerrekSB how would you inlcude the input operations in the function?

sehe Over a year ago

@Extinct23: if ((std::cin >> i) && (i>=0) && (i<=50)) -- but that would be rather sick, IMO. Code clarity above compact code

Brian Colvin Over a year ago

if ((std::max(usrInput1,usrInput2)<=50) && (std::min(usrInput1,usrInput2)>=0)){ //success } else {//fail} Ors are no fun!

|

Jerry Coffin · Accepted Answer · 2011-10-12 15:12:18Z

I think I'd move most of it into a function:

bool check_range(int input, 
                  int lower, int upper, 
                  std::string const &too_low, std::string const &too_high) 
{
    if (input < lower) {
        std::cerr << too_low << "\n";
        return false;
    }
    if (input > upper) {
        std::cerr << too_high << "\n";
        return false;
    }
    return true;
}

Then you'd use this something like:

if (check_range(usrInput1, 0, 50, "Error 1", "Error 2") &&
    check_range(usrInput2, 0, 50, "Error 3", "Error 4")
{
    // Both values are good
}

I should add that I'm not entirely happy with this. The function does really embody two responsibilities that I'd rather were separate (checking the range and reporting the error if its out of range).

Collectives™ on Stack Overflow

Simplifying if else user input validation

2 Answers 2

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related