Sorry I was thinking of throw std::bad_alloc

Why would you throw std::bad_alloc, the exception meant for memory allocation failure, for not being able to parse a string?

I think he just means throw an exception in general.

If one was to be thrown, an std::runtime_error would be better. Perhaps a custom 'my::bad_conversion' or 'my::bad_parse' exception used by all your conversion functions, that inherits from std::runtime_error.

However, passing in non-conforming data into StringToX type functions aren't really exceptional behavior, IMO - but I suppose that depends on the project.

That's really a horrible idea. If you're going to return a pair for error code and conversion result, then don't use dynamic allocation for it. Your own sample code leaks memory from using this kind of interface and calls the function twice to see if there's a conversion error.

You can also use String-Streams!

Cheers !

I already am to do the conversion? I thought I mentioned that in my first post.

If you just do the conversion with a stringstream and extract the number out of the string that isn't a pure number then the number will be filled with garbage

Quick example: if the the string we tried to convert was "abc20" then the number will be filled with garbage. To me that then would need to require a check to make sure the conversion is possible at first so the number isn't filled with garbage.

Also I really do agree with Servant in that a "StringToX" function really isn't exceptional behavior. Though it seems most of the library functions I see that do this throw an exception. I do have it working by just throwing an exception if it couldn't do the conversion. I then wrote another quick that just did the conversion and in a program would only do the conversion if the string was a number:

std::string myNumber = "20 abc";
int number = 0;

if(IsNumber(myNumber))
     number = ToNumber<int>(myNumber);


     

that way the conversion only happens if I know it is a number. Though while this may work just fine in my own little projects for a university, I don't think this is very safe for other users. It seems it would force the users to always check before they tried the conversion and would require the users to create their own way of dealing with and error like that.

Though maybe I'm over thinking it?

std::string myNumber = "20 abc";
int number = 0;

if(IsNumber(myNumber))
     number = ToNumber<int>(myNumber);

that way the conversion only happens if I know it is a number. Though while this may work just fine in my own little projects for a university, I don't think this is very safe for other users. It seems it would force the users to always check before they tried the conversion and would require the users to create their own way of dealing with and error like that.

Not to mention that for every conversion, you have to iterate over the string twice instead of validating and converting at the same time (Schlemiel the Painter's algorithm, almost).

T ConvertNumber(string numberStringToConvert, bool &wasANumber)

Return your number however you return it, and set your boolean to true if it was a number.

std::string myNumber = "20 abc";
int number = 0;

if(IsNumber(myNumber))
     number = ToNumber<int>(myNumber);

that way the conversion only happens if I know it is a number. Though while this may work just fine in my own little projects for a university, I don't think this is very safe for other users. It seems it would force the users to always check before they tried the conversion and would require the users to create their own way of dealing with and error like that.

Not to mention that for every conversion, you have to iterate over the string twice instead of validating and converting at the same time (Schlemiel the Painter's algorithm, almost).

Even though I didn't know a name for it I was thinking that it would have to iterate over the string twice.

T ConvertNumber(string numberStringToConvert, bool &wasANumber)

Return your number however you return it, and set your boolean to true if it was a number.

That's a solution that I really didn't think of. Thanks for listing another solution.

Ok, i just wanted to post another solution I did. What would you change? What should be better? I know it isn't nearly the best solution.

IsNumber Function

bool IsNumber(const std::string& text)
{
	// const iterator to iterate through the string
	std::string::const_iterator it = text.begin();

	std::locale loc;

	// while we have not reached the end of the string and the character we are on is a digit
	// negative sign, or decimal point
	while(it != text.end() && (std::isdigit(*it, loc) || *it == '-' || *it == '.'))
	{
		it++;
	}

	return !text.empty() && it == text.end();
}

ToNumber

// ToNumber
// Takes in a string to convert to a number
// Checks to make sure that the string is a number before converting
// sets validNumber to true if it can convert, false otherwise
// Return Type Element to convert the string to any number
template <typename Element>
Element ToNumber(const std::string& text, bool& validNumber)
{
	Element number = 0;

	// If the string is a number, validNumber is true, otherwise false
	(IsNumber(text)) ? (validNumber = true) : (validNumber = false);
	
	// Only do the conversion if we have a validNumber;
	if(validNumber)
	{
		// istringstream buffer to hold the string passed in
		std::istringstream buffer(text);
		// extract the number from the buffer
		buffer >> number;
	}

	return number;
}

I still feel it could be a lot better. I am still not a fan of the IsNumber Function. I feel it could be a bit too confusing to follow for something that is so simple.

So couple questions:

1: Better solution? As in am I still over thinking this?

2: is it right to use the c++ std::isdigit function here that takes the std::locale? I saw an example using it and after trying to read about the std::locale I wasn't 100% sure what I was doing exactly.

3: Bad use of the Conditional Ternary Operator? I know what it is saying just fine and I felt a full if/else statement was too long here for something really simple. Though would this be by chance considered abusing the operator?

Maybe I just need to get over always using the full if/else with the curly braces on something that simple (I for some reason do a double take every time the curly braces aren't used on those one liners if/else statements, yet I read the conditional operator just fine almost always).

First basic problem is that your IsNumber() function works the same for every single number type, but not every number type has the same valid format. What's a valid float is not necessarily a valid int. Secondly, you're only looking at valid characters without looking at the pattern of characters. IsNumber() would return true for "....." but this isn't a valid number for any data type. Next, if you're going to bring in a locale, then you should be looking at valid number formats for the given locales. For instance "10,000" is a valid formatted integer with a thousands separator for en-US, but the same number number formatted for de-DE is "10.000" neither of which would be parsed by std::stringstream properly as an integer. IsNumber() also misses the exponent portion of a float which generally starts with an "e" or "E". Also being a properly formatted number doesn't mean that the conversion would succeed. For instance, what happens when you want to format "10000000000000000000000000000" into a 32-bit int?

The ternary operator can be replaced very simply with validNumber = IsNumber(text);.

But again, the whole IsNumber() logic is simply duplicating the work std::stringstream already does when parsing the number itself. It would be less work, both in terms of programmer effort and in clock cycles at run time, and less bug prone to just to check the state of the stream after extraction.