Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion methods do not fail if the input string cannot be converted #6

Open
lahwaacz opened this issue Jan 31, 2020 · 6 comments
Open

Comments

@lahwaacz
Copy link

For example, if you have an option with a text, e.g. foo = bar, and use file.GetValue("foo").AsInt(), you get 0 (value of default-constructed int) instead of an error. There should be an exception thrown if the input value from the config does not contain the expected type.

@Lek-sys
Copy link
Owner

Lek-sys commented Apr 6, 2020

Class stores everything it reads from the file in strings, without typization. For the sake of stability, portability and easy of use, I dont wont the class to throw exceptions of any kind. I dont really think, that treating unexpected string as 0 should be a problem, but if in particular case it is, you can always get the original, unconverted string by calling Value::AsString() and do the convertion in top-level code.

@Lek-sys Lek-sys closed this as completed Apr 6, 2020
@lahwaacz
Copy link
Author

lahwaacz commented Apr 6, 2020

I dont really think, that treating unexpected string as 0 should be a problem

It is a problem because you don't know if the config really contains 0 or something invalid. If you don't have an API that allows to check errors, then it is pretty much useless for anything serious.

@Lek-sys
Copy link
Owner

Lek-sys commented Apr 21, 2020

You can always write something like this

namespace INI
{
	template<> inline int string_to_t<int>(const std::string& v) throw(...)
	{
		convstream ss;
		ss << v;
		int out = 0;
		ss >> out;
		if (out == 0 && v != "0")
			throw std::string("Invalid conversion!");
		return out;
	}
}

@lahwaacz
Copy link
Author

And do this for every type that is used with the string_to_t function? I'd rather fork your library and change the implementation of the function template myself if you are not willing to support this.

Btw. the proper way to check if the conversion failed is to check str.fail(). And of course throwing std::string as an exception is not a good idea.

@Lek-sys
Copy link
Owner

Lek-sys commented Apr 21, 2020

Yes, you are right, str.fail() is more universal method. As for throwing.. To my mind, throwing anything is not a very good idea if you write something, that should be portable and may end up in dynamic library.. I just add that as a quick and dirty example.
But, while writing this, i've decided, that adding optional bool* parameter to As<>() function to allow error reporting can be indeed a good idea. Thanks. I will reopen the issue.

@Lek-sys Lek-sys reopened this Apr 21, 2020
@lahwaacz
Copy link
Author

To my mind, throwing anything is not a very good idea if you write something, that should be portable and may end up in dynamic library..

You're contradicting the entire STL where most functions may throw exceptions - it is the most portable C++ library there is and often ends up in dynamic libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants