I'm working on a set of PostgreSQL helper classes, and am representing each PostgreSQL data type as objects. Instead of creating a class for each number format, I've created a template so the user can create a Number<int>
, Number<double>
etc as necessary, which is awesome as it saves me lots of coding!
The problem I'm facing is I have the need to convert a string representing the value of the number (which is returned from libpq) to its relevant format (the fromString
method).
- It's kinda messy. Is there a neater solution than a lot of if / else's?
- I also have the problem that if the string doesn't contain type
T
, it causes an exception, so, to be safe, I'd have to wrap everything inside a try/catch making it even more messier! So, is there a way around doing this?
template <typename T>
class Number : public spg::datatypes::BaseDataType
{
public:
Number() : BaseDataType(false) { checkTIsNumeric(); }
Number(T val) : BaseDataType(false), val(val) { checkTIsNumeric(); }
Number(const std::string& s) : BaseDataType(false) { checkTIsNumeric(); fromString(s); }
std::string toString() const { return std::to_string(val); }
int getParamCount() { return 1; }
T val;
protected:
void parseString(std::vector<std::string>& splitted)
{
// see http://www.cplusplus.com/reference/string/
size_t hc = typeid(T).hash_code();
// TODO maybe these should be wrapped inside try/catch because if string doesn't contain T, it will throw
if (hc == typeid(int).hash_code())
val = std::stoi(splitted[0]);
else if (hc == typeid(double).hash_code())
val = std::stod(splitted[0]);
else if (hc == typeid(long double).hash_code())
val = std::stold(splitted[0]);
else if (hc == typeid(float).hash_code())
val = std::stof(splitted[0]);
else if (hc == typeid(long).hash_code())
val = std::stol(splitted[0]);
else if (hc == typeid(unsigned long).hash_code())
val = std::stoul(splitted[0]);
else if (hc == typeid(long long).hash_code())
val = std::stoll(splitted[0]);
else if (hc == typeid(unsigned long long).hash_code())
val = std::stoull(splitted[0]);
}
private:
void checkTIsNumeric()
{
if (!std::is_arithmetic<T>::value || std::is_class<T>::value)
throw "T is not numeric";
}
};
I'm not sure if the checkTIsNumeric
is necessary, as compilation will fail if a non numeric type is passed as T
because std::to_string
doesn't have a suitable overload.
I did a few tests using the giant if / else above, and using a std::stringstream
. The latter was noticeably slower. E.g. in one benchmark, 14.0781s for the above code vs 19.6901s using std::stringstream
.
2 Answers 2
First things first: Export all the things which are different for different types into a policy-class, like
std::basic_string
and many standard-library classes do.
The main advantage is being actually able to have types with different interfaces without running into compilation-errors or at least warnings:template<class T> struct standard_number_traits { static T parse(const std::string& in) { T out; std::stringstream{in} >> out; return out; } using std::to_string; };
Add specializations as needed for efficiency / versatility / correctness / fun.
Eliminate runtime-checks for having selected an acceptable type, powered by the prior change.
That also allows you to use the same basic C++ type with completely different IO-behavior:template <class T, class traits_type = standard_number_traits<T>> struct Number : spg::datatypes::BaseDataType { Number(T val = T{}) : val(val) {} // Or is BaseDataType(false) not default-init? Number(const std::string& s) : Number(traits_type::parse(s)) {} std::string toString() const { return traits_type::to_string(val); } int getParamCount() { return 1; } T val; // Shall that really be public? Well, might be ok. };
- Profit?
-
\$\begingroup\$ You're right,
T val
shouldn't be public! A few quick operator overloads later, and it's now a proud protected! \$\endgroup\$user3791372– user37913722014年08月15日 08:28:33 +00:00Commented Aug 15, 2014 at 8:28 -
\$\begingroup\$ If the types are ever only to be used with one interface (user to set values, and values parsed to and from database), then does it still make sense to build a policy class? \$\endgroup\$user3791372– user37913722014年08月15日 08:30:38 +00:00Commented Aug 15, 2014 at 8:30
-
\$\begingroup\$ Yes, just to sparate the changing parts out. Well, you could certainly define the few functions it pertains to inline but outside the class... \$\endgroup\$Deduplicator– Deduplicator2014年08月15日 09:42:12 +00:00Commented Aug 15, 2014 at 9:42
Compilation error:
Number(const std::string& s) : BaseDataType(false) { checkTIsNumeric(); fromString(s); }
There is no fromString()
maybe you menat parseString()
Why does parseString()
take a vector?
void parseString(std::vector<std::string>& splitted)
You only every use the first element.
If you are not going to modify a parameter help the compiler by letting it know that the parameter is const (thus will not be modified). So pass the vector by const reference. If you remove the vector pass by const reference to std::string.
Currently parseString()
is implemented as a runtime check on the type. But you know the type at compile time. So you should use template specialization.
So in the class define a default version that does nothing. I would change that to try and use the constructor (that has a string). If the class does have a string constructor it is a compiler error but you probably want to know that,
class Number
{
// Yes I modified the type from your original.
void parseString(std::string const& splitted)
{
val = std::move(T(splitted));
}
};
Then you can define specializations:
// Here is the version for int.
template<>
void Number<int>::parseString(std::string const& splitted) {val = std::stoi(splitted);}
// Here is the version for float.
template<>
void Number<float>::parseString(std::string const& splitted) {val = std::stof(splitted);}
// etc ...
-
\$\begingroup\$
fromString
is a method of the base class, which splits the string, and for each string element passes it to a virtual methodparseString
(which should in hindsight be calledparseStrings
, or justparse
, which assigns local elements from the string values. E.g. apoint
data type will be passed a vector of strings with two elements, andx,y
will be assigned from the values \$\endgroup\$user3791372– user37913722014年08月15日 08:10:39 +00:00Commented Aug 15, 2014 at 8:10 -
\$\begingroup\$ Really like the idea of template specialisations. It's what I should have done in the first place! \$\endgroup\$user3791372– user37913722014年08月15日 08:41:53 +00:00Commented Aug 15, 2014 at 8:41
std::stringstream
orboost::lexical_cast
. \$\endgroup\$std::to_string
which is a C++11 feature, I would recommend moving your type checking from run time, using exceptions, to compile time usingstatic_assert
. \$\endgroup\$std::stringstream
? that would be for number to string, no? not from string to number? Besides, I would have thought usingval = std::stox(input)
would be faster than creating astd::stringstream
object and then calling methods on it? \$\endgroup\$std::stringstream
can be used both ways. You can set a string on it and read a value as you would read fromstd::cin
. \$\endgroup\$static_assert
usually produces better error messages and should be preferred. \$\endgroup\$