6
\$\begingroup\$

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).

  1. It's kinda messy. Is there a neater solution than a lot of if / else's?
  2. 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.

\$\endgroup\$
  • 3
    \$\begingroup\$ I'd suggest looking into either std::stringstream or boost::lexical_cast. \$\endgroup\$ – Yuushi Aug 14 '14 at 14:25
  • 2
    \$\begingroup\$ Since you are already using std::to_string which is a C++11 feature, I would recommend moving your type checking from run time, using exceptions, to compile time using static_assert. \$\endgroup\$ – Tiago Gomes Aug 14 '14 at 14:33
  • \$\begingroup\$ @Yuushi std::stringstream? that would be for number to string, no? not from string to number? Besides, I would have thought using val = std::stox(input) would be faster than creating a std::stringstream object and then calling methods on it? \$\endgroup\$ – user3791372 Aug 14 '14 at 14:34
  • 1
    \$\begingroup\$ @user3791372 std::stringstream can be used both ways. You can set a string on it and read a value as you would read from std::cin. \$\endgroup\$ – Tiago Gomes Aug 14 '14 at 14:39
  • 1
    \$\begingroup\$ @Deduplicator SFINAE is useful when removing functions from overload resolution as other overloads may be able to provide the functionality this one can't. For classes it doesn't make much sense as you can't "overload" classes and you really just want to make it an error. For such, static_assert usually produces better error messages and should be preferred. \$\endgroup\$ – Tiago Gomes Aug 14 '14 at 22:22
4
\$\begingroup\$
  1. 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.

  2. 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.
    };
    
  3. Profit?
\$\endgroup\$
  • \$\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 Aug 15 '14 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 Aug 15 '14 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 Aug 15 '14 at 9:42
3
\$\begingroup\$

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 ...
\$\endgroup\$
  • \$\begingroup\$ fromString is a method of the base class, which splits the string, and for each string element passes it to a virtual method parseString (which should in hindsight be called parseStrings, or just parse, which assigns local elements from the string values. E.g. a point data type will be passed a vector of strings with two elements, and x,y will be assigned from the values \$\endgroup\$ – user3791372 Aug 15 '14 at 8:10
  • \$\begingroup\$ Really like the idea of template specialisations. It's what I should have done in the first place! \$\endgroup\$ – user3791372 Aug 15 '14 at 8:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.