Please tell me how bad this code segment is. I figured I would share this horrible code. If it's not as horrible as I think it is, please let me know. Thanks!

/*
		Purpose: Create a function to check the input from user. Only valid integer should be returned. All other input should be tossed, user should be asked again for valid integer input.
		Name: Saith
		Date: 6/4/11
*/
#include<iostream>
using namespace std;

int ReturnChosenNumber();

  int main(){

	  int number = ReturnChosenNumber();
	  cout << number;
		
			
return 0;


      }

  int ReturnChosenNumber(){

	  // A very ghetto function. Enjoy the scrap work, make shift. integer checker :D
	
	int number;
	cin >> number;
	
	while(cin.fail() ){
		cin.clear();
		cin.ignore(1000,'\n');
		cout << "Input not an integer value. Try again.\n";
		number = ReturnChosenNumber();
	}
	return number;
  }

Edit: I changed from using SStream, String, to just cin. The older version used them, but decided cin was just as good.

Recommended Answers

All 10 Replies

Odd. The above code seemed to work in a small test program, but once I tried it in my project, it did not work as expected. I changed it slightly to what I've linked below. Seems to work fine now.

/*
    Purpose: Create a function to check the input from user. Only valid integer should be returned. All other input should be tossed, user should be asked again for valid integer input.
    Name: Saith
    Date: 6/4/11
    */
    #include<iostream>
    using namespace std;
     
    int ReturnChosenNumber();
     
    int main(){
     
      int number = ReturnChosenNumber();
      cout << number;
     
     
      return 0;
    }
     
    int ReturnChosenNumber(){
     
     // A very ghetto function. Enjoy the scrap work, make shift. integer checker :D
     
     int number;
     cin >> number;
     
     while(cin.fail() ){
      cin.clear();
      cin.ignore(1000,'\n');
      cout << "Input not an integer value. Try again.\n";
      cin >> number;
     }
     return number;
    }

Use the second one. The first one is crap.

Personally I would rather not deal with "cin" failure, but rather a stringstream failure.

I'm writing this in the textbox but it usually works when I do:

template<class int_type>
int_type GetChosenNumber(std::istream &is, std::ostream &os){
   int_type out;
   std::string in;
   std::getline(is, in);
   while( (std::stringstream(in) >> out).fail() ){
      os << "Invalid input, retry." << std::endl;
      std::getline(is,in);
   }
   return out;
}

Use is like:

//Assuming the compiler determines the type.
int result = GetChosenNumber(cin,cout);
//Assuming the compiler doesn't determine the type.
int result = GetChosenNumber<int>(cin,cout);
//Output to a file.
int result = GetChosenNumber<int>(cin,someOutFile);
//Floating point type.
float result = GetChosenNumber(cin,cout);

An overload to use your own error message might be handy too. Note that cin never enters the fail state with this, the stringstream does and a new one is created every time the parsing fails, so you don't have to clear errors and ignore stuff (which is kind of an iffy subject for different platforms).

@OP or a more generic approach:

struct InvalidInputException : std::exception{
 InvalidInputException(): std::exception("Error: Invalid conversion") {}
};

//tries to get a data of type T, throws InvalidInputException if failed
template<typename T>
T prompt(const std::string msg){ 
  cout << msg ;
  T res = T();
  if( ! (cin >> res) ){ 
      cin.clear();
      while(cin.get() != '\n' ) continue;
      throw InvalidInputException();
  }
 else return res;
}

int main(){
 try{
   int age = prompt("What is your age? ");
   string name = prompt("What is your name? ");
  }catch(std::exception& e){ cout << "whyyyyyyy?\n"; }
}

@psuedorandom: I like that solution but some things,

1) The name int_type is misleading since the user can use that function to get a string for example.
2) I don't like the performance issue with writing just one input to a file everytime that function is called, I would rather do lazy write and write only when necessary.


and I guess whether the function should throw exception on failed input is a matter of choice.

[B]>[/B] int number;
[B]>[/B] stm >> number;
[B]>[/B] while( stm.fail() ) ....

This kind of construct fails to check for crud remaining in the input; 567#$! would be treated as a valid number with a value of 567.

If the attempt to read a non-ws char after reading the number is successful, there was some crud left in the input line.

#include <type_traits>
#include <iostream>
#include <sstream>

template< typename T = int >
T number_from_stdin(  const char* prompt = "enter a valid number, then a <newline>: ",
                      const char* emsg = "error: input is not numeric\n",
                      typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0  )
{
    std::string str ;
    std::cout << prompt ;
    if( std::getline( std::cin, str ) )
    {
        std::istringstream stm(str) ;
        T number ;
        char trailing_crud ;
        if( ( stm >> number ) && !( stm >> std::ws >> trailing_crud ) ) return number ;
    }
    std::cerr << emsg ;
    return number_from_stdin<T>( prompt, emsg ) ;
}

Yes, good point Vijayan, I usually assume it's good input even if it has trailing crud for some reason.

Might I ask for an explanation of this line:

typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0 )

> Might I ask for an explanation of this line
> typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0 Well, the original question was about

Only valid integer should be returned. All other input should be tossed, user should be asked again for valid integer input.

I took the liberty of generalizing that to 'Only valid number should be returned. All other input should be tossed, user should be asked again for valid numeric input.'. Where the number is any numeric (integral or real) type.

So, the template function

template< typename T = int > T number_from_stdin( ... )

should only be available for numeric types. The last parameter to the function is just a place holder with a default value to ensure that T is a numeric type.

An instance of std::is_arthmetic<T> holds true only if the type T is an arithmetic type (either an integral type or a floating point type). std::is_arithmetic<volatile int>::value is true , so an improved version of the function would be:

template< typename T = int >
T number_from_stdin(  const char* prompt = "enter a valid number, then a <newline>: ",
                      const char* emsg = "error: input is not numeric\n",
                      typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0  )
{
    std::string str ;
    std::cout << prompt ;
    if( std::getline( std::cin, str ) )
    {
        std::istringstream stm(str) ;
        [B]typename std::remove_cv<T>::type number ;[/B]
        char trailing_crud ;
        if( ( stm >> number ) && !( stm >> std::ws >> trailing_crud ) ) return number ;
    }
    std::cerr << emsg ;
    return number_from_stdin<T>( prompt, emsg ) ;
}

std::enable_if<> proposal was based on boost::enable_if<> , which is explained here: http://drdobbs.com/cpp/184401659

Thank you Vijayan121, that is very helpful.

> Might I ask for an explanation of this line
> typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0 Well, the original question was about

Only valid integer should be returned. All other input should be tossed, user should be asked again for valid integer input.

I took the liberty of generalizing that to 'Only valid number should be returned. All other input should be tossed, user should be asked again for valid numeric input.'. Where the number is any numeric (integral or real) type.

So, the template function

template< typename T = int > T number_from_stdin( ... )

should only be available for numeric types. The last parameter to the function is just a place holder with a default value to ensure that T is a numeric type.

An instance of std::is_arthmetic<T> holds true only if the type T is an arithmetic type (either an integral type or a floating point type). std::is_arithmetic<volatile int>::value is true , so an improved version of the function would be:

template< typename T = int >
T number_from_stdin(  const char* prompt = "enter a valid number, then a <newline>: ",
                      const char* emsg = "error: input is not numeric\n",
                      typename std::enable_if< std::is_arithmetic<T>::value, T >::type* = 0  )
{
    std::string str ;
    std::cout << prompt ;
    if( std::getline( std::cin, str ) )
    {
        std::istringstream stm(str) ;
        [B]typename std::remove_cv<T>::type number ;[/B]
        char trailing_crud ;
        if( ( stm >> number ) && !( stm >> std::ws >> trailing_crud ) ) return number ;
    }
    std::cerr << emsg ;
    return number_from_stdin<T>( prompt, emsg ) ;
}

std::enable_if<> proposal was based on boost::enable_if<> , which is explained here: http://drdobbs.com/cpp/184401659

wait what does enable_if return if false? if true then ::type* will be for example int*=0 which compiles but if false then what is ::type? just a false value?

> wait what does enable_if return if false?

Repeat: std::enable_if<> proposal was based on boost::enable_if<>, which is explained here: http://drdobbs.com/cpp/184401659

Alternatively, look up section 20.9.7.6 in the FDIS.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.