Thread: Advice on basic number validation

I have been experimenting with basic number validation and i need a little advice...

Code:

#include <iostream>

using namespace std;

int main() {
 int num;

 start:

 cout << "Please enter a Number between 1 and 10... ";
 cin >> num;
 if (num < 1 || num > 10) {
  cout << "Invalid Number" << endl;
  goto start;
 }
 else {
  cout << "Valid Number";
 }

 cin.ignore();
 cin.get();
}

Is this the right approach for validating a number? How would i validate that it's a number and not any other character? I noticed that if i enter a letter instead at run time the console closes presumably crashing the program.

And finally, have i looped correctly? If an invalid number is entered it needs to ask for a number to be inputted again, I seem to recall reading that it's not advisable to use goto anymore, but I'm not sure how else to do it.

I would appreciate any advice you can give me

I like to read numbers in as strings, validate and then convert them to whatever I need.

Thank you.

Could you confirm i have understood this line correctly from the FAQ:

Code:

 if ( scanf ( "%d", &num ) != 1 )

The condition is that scanf did not successfully read one item and
scanf itself attempts to read inputted data and format is as an integer?

I don't fully understand the need for the address operator, i presume it's purpose is to specify the memory location to write the value to but can it not simply assign the value to the variable?


EDIT: Thank you Lithorien, i will have a look through your example now

Originally Posted by Diablo84

Thank you.

Could you confirm i have understood this line correctly from the FAQ:

Code:

 if ( scanf ( "%d", &num ) != 1 )

The condition is that scanf did not successfully read one item and
scanf itself attempts to read inputted data and format is as an integer?

I don't fully understand the need for the address operator, i presume it's purpose is to specify the memory location to write the value to but can it not simply assign the value to the variable?


EDIT: Thank you Lithorien, i will have a look through your example now

scanf() is not a C++ function, you're much better served to use cin(). However:

What you are saying in that statement is "If scanf(interger, store_in_num) != One_Int_Read), if I remember my C correctly. To put it in more human terms, you are checking to see if scanf() (which reads an interger into num) only got 1 int from the user.

So yes, you were right about the condition being that scanf() didn't read just one item.

scanf() also stores the input into addresses - that's why you have to give it &num and not just num.

OP: You approach isn't going to get you where you need to go. Take a look at this and see if it helps.



This will work if the user enters a char and not a number, as well as if the user enters a number.

Code:

#include <iostream>

int main()
{
	bool valid = 0;
	
	do
	{
		char *ans = new char;
		int num = -1;
		
		std::cout << "Input a number (0-9): ";
		std::cin >> ans;
		
		if (isdigit(*ans))
		{
			num = atoi(ans);
		}
		
		if ((num < 0) || (num > 9))
		{
			std::cout << "\nInvalid number.\n\n";
		}
		
		else
		{
			std::cout << "\nYour number was: " << num << ".";
			valid = 1;
		}
	}
	while (valid == 0);
	
	std::cin.ignore(80, '\n');
	std::cin.get();
	return(0);
}

Originally Posted by Lithorien

OP: You approach isn't going to get you where you need to go. Take a look at this and see if it helps.



This will work if the user enters a char and not a number, as well as if the user enters a number.

Code:

#include <iostream>

int main()
{
	bool valid = 0;
	
	do
	{
		char *ans = new char;
		int num = -1;
		
		std::cout << "Input a number (0-9): ";
		std::cin >> ans;
		
		if (isdigit(*ans))
		{
			num = atoi(ans);
		}
		
		if ((num < 0) || (num > 9))
		{
			std::cout << "\nInvalid number.\n\n";
		}
		
		else
		{
			std::cout << "\nYour number was: " << num << ".";
			valid = 1;
		}
	}
	while (valid == 0);
	
	std::cin.ignore(80, '\n');
	std::cin.get();
	return(0);
}

I just feel I need to point this out.

Your approach opens for alot of errors.
First:
you use new but never call delete, that means you have created a memoryleak. Also, that new is totally uncalled for.

Second:
std::cin >> ans;
should be std::cin >> *ans; since we are dealing with pointers.

Third:
atoi expects an array of chars, you send it a pointer to one char. This I suspect can result in undefined behavour (sp?). Either way you should send a null-terminated string to atoi.

Sorry if it seems like im whining, I just think these things are important enough to point out.

Originally Posted by Shakti

I just feel I need to point this out.

Your approach opens for alot of errors.
First:
you use new but never call delete, that means you have created a memoryleak. Also, that new is totally uncalled for.

The new has a place. Try it with just a normal char and you introduce a lot of pain -- however you're right, I did forget delete. Thank you.

Originally Posted by Shakti

Second:
std::cin >> ans;
should be std::cin >> *ans; since we are dealing with pointers.

No it doesn't. You're dealing with a pointer, yes, but if you just go >> ans, you'll be storing in the memory pointed to by ans anyway.

Originally Posted by Shakti

Third:
atoi expects an array of chars, you send it a pointer to one char. This I suspect can result in undefined behavour (sp?). Either way you should send a null-terminated string to atoi.

Sorry if it seems like im whining, I just think these things are important enough to point out.

atoi() is recieving the value pointed to by ans, not a pointer to ans. Take a look at isdigit() -- THAT is recieving a pointer.

Yet again, however, good point about the non-null termination. Thank you for that - just change 'new char' to 'new char[255]', and any reference to 'ans' with 'ans[0]'.

@Micko: Maybe:

Code:

#include <iostream>

int main() 
{
    using namespace std;
    
    int num;
    
    for(;;)
    {
           cout << "Please enter a Number between 1 and 10... ";
           cin >> num;
		   if (cin.fail())
		   {
			   cout << "Must be a number!!!\n" << endl;
			   break;
		   }
		   else
		   {
				if ( (num >= 1) && (num <= 10) ) 
				{
					cout << "Valid Number\n";
					break;
				}
				cout << "Invalid Number" << endl;
				continue;
		   }
    }
    system("pause"); // not portable
    return 0; 
}

The *Goto statement* is taboo for most programmers.

Wouldn't use it- it's bad. Use a while loop with a condition

Code:

   //The start of your program

    int loop=0;
    while (loop==0)
    {
        cout<<"Enter:"<<endl;
        cin>>Enter;

          if (Enter==valid)
          {
           do stuff;
           condition==1;  //Thus breaking the while condition
           }
     
           if (Enter==invalid)
          {
          then condition is still 0 so will loop to beginning
           }
   
    }

Perhaps?

I have just been doing some reading on why it's advisable to avoid goto. Initially i got the impression that it was just taboo but for no specific reason. After digging a little deeper i found it's association with spaghetti code in a poorly written program etc. It's also clear now that in the vast majority of cases there is always a better alternative.

The only query left in my mind now is, what would happen in a situation where there was a potential good use for goto. ie. it would be a more simple, efficient and effective approach to any alternative. Would it be acceptable to use a goto statement then or is it a big no no which should always be avoided?