7
\$\begingroup\$

Following on from my earlier question: Very simple PostgreSQL ORM in C++ using libpq (tldr; I'm creating a set of helper classes for working with libpq)

I've decided to have another attack at the datatypes which represent postgresql data types so some sort of ORM can occur with my classes. Following on from the advice in the question, this is what I've come up with. I'm trying to make the base class a bit more powerful and to save having to repeat code for each derived class.

class BaseDataType
{
public:
    BaseDataType(bool isCompound) : isCompound(isCompound) {}
    virtual ~BaseDataType() {}

    virtual std::string toString() const = 0;

    // return the amount of params that should be in the string for this data type
    // this is used in the fromString method to check that the correct # of params are included in the string
    virtual int getParamCount() = 0;

protected:
    bool isCompound = false;
    void fromString(std::string s) 
    {
        // remove all parenthesis and split on ','
        spg::convert::ReplaceAll(s, "(", "");
        spg::convert::ReplaceAll(s, ")", "");
        std::vector<std::string> splitted = split(s, ',');

        if (splitted.size() != getParamCount()) throw "Input string doesn't have enough params";

        // each data type will implement its own parseString method that uses the strings in the splitted array
        // therefore it's fairly essential that each datatype has a constructor that allows each param to be a string
        // TODO this kinda feels a bit messy?
        parseString(splitted);
    }

    // the method that would take the array of strings and populate local members
    virtual void parseString(std::vector<std::string>& splitted) = 0;

};

class Point : public BaseDataType
{
public:
    Point() : BaseDataType(false) {}
    Point(double x, double y) : BaseDataType(false), x(x), y(y) {}
    Point(const std::string& s) : BaseDataType(false) { fromString(s); }
    Point(std::string& x, std::string& y) : BaseDataType(false) { fromString(x + "," + y); }

    std::string toString() const { return std::to_string(x) + "," + std::to_string(y); }
    int getParamCount() { return 2; }

    double x, y;

protected:
    void parseString(std::vector<std::string>& splitted)
    {
        x = std::stod(splitted[0]);
        y = std::stod(splitted[1]);
    }

};

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

    Box(const std::string& s) : BaseDataType(false) { fromString(s); }
    Box(const std::string& corner1, const std::string& corner2) : BaseDataType(false) { fromString(corner1+","+corner2); }

    std::string toString()const { return corner1.toString() + "," + corner2.toString(); }
    int getParamCount() { return 4; }

    Point corner1, corner2;

protected:
    void parseString(std::vector<std::string>& splitted)
    {
        corner1 = Point(splitted[0],splitted[1]);
        corner2 = Point(splitted[2],splitted[3]);
    }
};

// a postgres compound data type: CREATE TYPE FOO AS (angle REAL, a_box Box);
class SomeCompoundType : public BaseDataType
{
public:
    SomeCompoundType() : BaseDataType(true){}
    SomeCompoundType(double angle, Box box) : BaseDataType(true), angle(angle), box(box){}
    SomeCompoundType(const std::string& s) : BaseDataType(true) { fromString(s); }
    SomeCompoundType(const std::string& angle, std::string& box) : BaseDataType(true) { fromString(angle + "," + box); }

    // this needs parenthesis around the outer type as it's a compound string
    // TODO I need to work this into the base maybe
    std::string toString() const { return "(" + std::to_string(angle) + "\(" + box.toString() + "\))"; }
    int getParamCount() { return 5; }

    double angle;
    Box box;
protected:
    // example "(2.35, (10,10,20,25))"
    void parseString(std::vector<std::string>& splitted)
    {
        angle = std::stod(splitted[0]);

        // inside a compound datatype, postgres needs parenthesis around data types that have more than one value
        box = Box(Point(splitted[1], splitted[2]), Point(splitted[3], splitted[4]));
    }

};

The split methods:

std::vector<std::string> &split(const std::string &s, char delim, std::vector<std::string> &elems) {
    std::stringstream ss(s);
    std::string item;
    while (std::getline(ss, item, delim)) {
        if (!item.empty())
            elems.push_back(item);
    }
    return elems;
}

std::vector<std::string> split(const std::string &s, char delim) {
    std::vector<std::string> elems;
    split(s, delim, elems);
    return elems;
}

and a basic implementation of the data types:

Point p("10,20");
Box b("1,2,3,4");
SomeCompoundType sct1(2.5, Box(1, 2, 3, 4));
SomeCompoundType sct2("(2.5, \(1,2,3,4\))");

I'm a bit concerned about the growth of constructors for each data type, with a minimum 4 necessary for each type (I'm including a default constructor as, I assume, good practice). Are there any ways around this growth?

\$\endgroup\$
5
  • \$\begingroup\$ Are you sure you're not reinventing any wheels here? Have you looked into libpqtypes and libpqxx ? \$\endgroup\$ Aug 12, 2014 at 13:13
  • \$\begingroup\$ As for growth of ctors: Personally, in your position I'd be doing as much of this as possible using either C++ metaclasses (i.e. template hacks) or code generation - either way, based on pg_catalog.pg_cast, pg_catalog.pg_types, etc, or the underlying header files they're generated from. Hand-coding it all is going to suck when you're working with user-defined types (CREATE TYPE ... AS), domains, extension types, etc. \$\endgroup\$ Aug 12, 2014 at 13:15
  • \$\begingroup\$ @CraigRinger Apparently libpqxx isn't too hot on bug fixing - it's been a while since it was last updated, as for libpqtypes, I'd rather have something more C++'ish with objects than C-like. As for user-defined types (compound types) they consist of a selection of postgresql data types. I may have a rethink about creating compound types, to prevent having to create all constructors as they will be already in place \$\endgroup\$ Aug 12, 2014 at 16:45
  • \$\begingroup\$ If it isn't so hot on bug fixing might it not be better to contribute / help maintain and improve rather than reinvent? As for libpqtypes - possibly still useful to wrap it / extend libpqxx to benefit from it. \$\endgroup\$ Aug 13, 2014 at 0:01
  • \$\begingroup\$ nope and nope unfortunately \$\endgroup\$ Aug 13, 2014 at 10:46

1 Answer 1

1
\$\begingroup\$

Is it expected that an object's isCompound could change after it's constructed? I think that member ought to be const - or better still, use an intermediate class:

class CompoundDataType : public BaseDataType

I don't see where this value is ever used, so perhaps it could be removed entirely?

The empty destructor can be explicitly defaulted:

virtual ~BaseDataType() = default;

The default constructors for the subclasses fail to initialise their members. Either provide defaults for these members or remove the default constructors.

I don't see why getParamCount() should modify an object, nor why its result could be negative:

virtual std::size_t getParamCount() const = 0;

The subclasses should use the override keyword to avoid accidentally defining new functions.

parseString() does more than its name suggests - it updates the contents from the string, so it could do with a better name. I guess it's a choice whether it's allowed to modify the vector contents; since we know where it's called from, we can add a comment there to clarify that the vector could changed by the call.

   if (splitted.size() != getParamCount()) throw "Input string doesn't have enough params";

Firstly, prefer to throw a subclass of std::exception rather than a string literal. Secondly, the string is inaccurate: the string might instead have too many values.

Constructors ought to delegate where possible, rather than all having to use the same base-class constructor:

Box()
  : BaseDataType{false}
  {}
Box(Point corner1, Point corner2)
  : Box{},
    corner1{std::move(corner1)},
    corner2{std::move(corner2)}
  {}
Box(double x1, double y1, double x2, double y2)
  : Box{Point{x1, y1}, Point{x2, y2}}
  {}

Box(const std::string& s)
  : Box{}
  { fromString(s); }
Box(const std::string& corner1, const std::string& corner2)
  : Box{corner1+","+corner2}
  {}

Instead of the proliferation of constructors, consider instead a Factory object that knows how to convert types, and can pass through objects that need no conversion:

class DataObjectFactory
{
public:
    Point makePoint(Point p) { return p; }
    Point makePoint(auto x, auto y)
    {
        return Point{makeDouble(std::move(x)), makeDouble(std::move(y))};
    }

    Box makeBox(Box b) { return b; }
    Box makeBox(auto corner1, auto corner2)
    {
        return Box{makePoint(std::move(corner1)), makePoint(std::move(corner2))};
    }
};

The Factory can then be responsible for splitting string inputs and marshalling the results to the appropriate functions.

\$\endgroup\$

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.