Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm working on a set of helper classes for working with libpq in C++ and I want to be able to pass in objects to the helper classes and have them internally represented as strings when sent to the db, and convert from string to objects when fetching data. I'm not particularly after a crazy ORM allowing custom objects, only the known PostgreSQL data types will suffice.

So, I'm thinking about a class for each data type, and each class managing the conversion to and from strings, using a method for each data type, toString() and fromString() because at the moment, data that is sent to the insert methods for inserting into the database is in the form of strings.

The problem I face now - which gets rather messy - is how to convert from string to these objects, when for example, a postgresql box is returned as (x1,y1),(x2,y2) but could be represented by the user as x1,y1,x1,y1, (x1,y1,x2,y2), (x1,y1),(x2,y2) or ((x1,y1),(x2,y2)) but I'm assuming that simple regex will suffice there?

    class IDataType
    {
    public:
        virtual std::string toString() = 0;

        std::string toEscapedString()
        {
            std::string s = toString();
            spg::convert::ReplaceAll(s, "(", "\"(");
            spg::convert::ReplaceAll(s, ")", ")\"");

            // as replace all add's a backslash to the start and end of the string if compound type
            if (isCompound)
                s = s.substr(1, s.length() - 2);

            // remove double \"(\"( and )\")\" caused by compound types
            spg::convert::ReplaceAll(s, "\"(\"(", "\"(");
            spg::convert::ReplaceAll(s, "\")\")", "\")");

            return s;
        }

    protected:
        IDataType(bool isCompound) : isCompound(isCompound) {}
        bool isCompound = false;
    };

    // helper class for numeric types
    // limit to int,float,double etc
    template <typename T>
    class Number : public IDataType
    {
    public:
        Number(T val) : IDataType(false), val(val)
        {
            // some dirty type checking for numbers
            if (!std::is_arithmetic<T>::value)
                throw "Arithmetic isn't possible, therefore type not numerical";

            else if (std::is_class<T>::value)
                throw "type is a class";
        }

        std::string toString()
        {
            return spg::convert::numberToString(val);
        }

        T val;
    };

    class Point : public IDataType
    {
    public:
        Point(double x, double y) : IDataType(false), x(x), y(y) {}

        std::string toString()
        {
            return x.toString() + "," + y.toString();
        }

        // see http://www.postgresql.org/docs/9.3/static/datatype-geometric.html
        // takes format (x,y) or x,y
        void fromString(std::string str)
        {
            // ...
        }

        Number<double> x, y;
    };

    class Box : public IDataType
    {
    public:
        Box(double x1, double y1, double x2, double y2) : IDataType(false), corner1(x1, y1), corner2(x2, y2) {}
        Box(Point corner1, Point corner2) : IDataType(false), corner1(corner1), corner2(corner2) {}

        std::string toString()
        {
            return corner1.toString() + "," + corner2.toString();
        }
        Point corner1, corner2;
    };

I'm implementing like so:

spg::SimplePg conn;

bool connected = conn.connect("dbname=db_test user=username password=123");
if (!connected) throw "not connected";

spg::datatypes::Point pt1(100, 300);    
spg::datatypes::Point pt2(200, 300);

std::vector<std::string> params;
params.push_back(pt1.toString()); 
params.push_back(pt2.toString());
conn.execParams("insert into test(a_point1, a_point2) values ($1::Point, $2::Point)", params);

For completeness, the conversion methods:

    std::stringstream ss;

    // for replace string values - http://stackoverflow.com/questions/2896600/how-to-replace-all-occurrences-of-a-character-in-string
    void ReplaceAll(std::string& str, const std::string& from, const std::string& to) {
        size_t start_pos = 0;
        while ((start_pos = str.find(from, start_pos)) != std::string::npos) 
        {
            str.replace(start_pos, from.length(), to);
            start_pos += to.length(); // Handles case where 'to' is a substring of 'from'
        }
    }

    // TODO there surely has to be a better way than this to convert to string?
    // TODO maybe I can just do this with generics?
    std::string numberToString(int i)
    {
        ss.str("");
        ss << i;
        return ss.str();
    }
    std::string numberToString(double d)
    {
        ss.str("");
        ss << d;
        return ss.str();
    }
    std::string numberToString(float f)
    {
        ss.str("");
        ss << f;
        return ss.str();
    }
  1. Is there a neater C++ way to manage the conversion to and from strings for the data types, maybe using generics?
  2. Conversion of a number to string feels messy. Is there a faster/neater way?
share|improve this question

1 Answer 1

up vote 3 down vote accepted

Firstly, and most importantly, you're missing a virtual destructor for your IDataType definition. This is bad, and should be the first thing you fix:

class IDataType
{
    virtual ~IDataType() { }
}

For 1, you could do this using templates. However, you don't have to, assuming you have access to C++11. There is a free function defined in <string> called to_string that has an overload for most of the basic numeric types. Hence any place you're calling one of your numberToString methods, you could simply replace this with std::to_string(...) (see here for the documentation of to_string).

Converting back the other way is probably going to require a bit more effort, however. You could potentially do this with regex (but note that creating a regex that will accept only integers or doubles is somewhat difficult and error prone; cases like 1. or .4 or 1e5 are very easy to overlook and fiddly to get right). In this case, the only real difference seems to be that there are (multiple) parens or no parens. Perhaps a better way would be to reduce it to a format with no parens, and then parse that:

class Point : public IDataType
{
    void fromString(std::string s)
    {
        // Remove parenthesis, leaving (hopefully) only "x,y"
        s.erase(std::remove_if(s.begin(), s.end(), 
                   [](char c) { return c == '(' || c == ')'; },
                s.end());
        // Split on ","
        std::vector<std::string> split;
        boost::split(split, s, boost::is_any_of(","));
        // Make sure there are only 2 values
        // May also want to do some checking around lexical_cast in case it fails
        x = Number(boost::lexical_cast<double>(split[0]));
        y = Number(boost::lexical_cast<double>(split[1]));
     }

     ....
 }

A very similar algorithm can be used for Box (N.B. I haven't tried to compile the above code, so I apologize in advance if there are any errors in it. Hopefully you get the general idea, though).

On a separate note, I'm not a huge fan of the design. The idea seems to be you can create an IDataType object which is mutated by calling fromString. Given the small size of these in general (and unless you really need this to be high performance), you might want to consider making them immutable:

class Point : public IDataType
{
    public:

         Point(double x, double y) 
           : IDataType(false), 
             x(x), 
             y(y) 
         { }

         Point(const std::string& s)
         {
             fromString(s);
         }

         std::string toString(....) const { } // Note: your toString method should be const

     private:

         void fromString(std::string s)
         {
             // Logic
         }
     };

This has a few benefits:

  1. Easier to reason about; once you've instantiated it with given parameters, you don't need to worry about things changing through a call to fromString.
  2. Easier to deal with exceptions: everything is done through the constructor, so you either get back a fully formed correct object, or the constructor throws. Currently, you need to make sure that your fromString function doesn't leave the object in an inconsistent state if it throws.
share|improve this answer
    
Lots of ideas! A few comments: 1/ lack of virtual destructor is bad due to derived class destructor not called if IDataType *ptr referenced a derived class. 2/ Converting to string from numbers with std::to_string is perfect! 3/ Although postgres accepts a variety of different string formats for datatypes, it always returns one string format. I think at this stage, it'll be easier to only convert fromString for that one string format - notable exception and one you highlight - 0.00001 / 1e-5 is returned as 1e-005. Thanks for tips and review. I'll work them into my code - I've learnt a lot! –  user3791372 Aug 11 at 17:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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