Datastore mapper class design

[o3hC5Di1]

Hi everyone,

For a library I'm writing I'm trying to set up a base "user" class which could form a basis for, for example, a user registered on a website.

struct base_user 
{
    int id;
    std::string username, password;
};

I use an adapter-pattern for mapping the users data to a datastore, to separate the type of datastore from the user-implementation:

struct base_user_mapper_interface
{
    virtual void read(base_user& u)=0;
    virtual void create(base_user& u)=0;
    //etc.
};


struct base_user_sql_mapper : public base_mapper_interface
{
    void read(base_user& u)
    {
        fetch("SELECT * FROM users WHERE id=?", u.id);
        //etc..
    }

    void create(base_user& u)
    {
        execute("INSERT INTO users(name, password) VALUES(?,?)", u.name, u.password);
        //etc.
    }
};

So here is my problem: This is supposed to be part of a library, making it easy for me to create new types of users, for example:

struct my_user : public base_user
{
    std::string email, address;
};

Now, when I want to create a new user in the datastore I would have to make a new mapper class as such:

struct my_user_sql_mapper : public base_user_sql_mapper
{
    void read(my_user& u)
    {
        base_user_sql_mapper::read(u);
        fetch("SELECT email, address FROM my_users WHERE id=?", u.id);
        //etc.
    }

    void create(my_user& u)
    {
        base_user_sql_mapper::create(u);
        execute("INSERT INTO my_users(email, password) VALUES(?,?)", u.email, u.password);
    }
};

However, that means that for every operation on the datastore, I need to connect twice to the datastore and perform actions on it. From an OO standpoint, to me, this seems the most logical solution, but from a datastore standpoint, it seems terribly inefficient.

Any advice on alternative solutions / design patterns would be much appreciated (even if it's just that this performance hit is something to live with).

Sorry for the long post, I like to be thorough.

Thanks in advance,
NwN

[rmxhaha]

I know almost all basic SQL and I know C++...
I don't really get what your code is trying to do in read function that is

why read if you don't return ...
if that your plan from the start
then you must have stored the data somewhere within the class

hmm, the thing about fetching the data twice has happened to me once and I come up with an ugly solution IMO

class my_user {
     void read(){
          fetch("QUERY");
          while( data != NULL ){
                process_base( data );
                process_user_specific_stuff( data );
                data = fetch_next_data();
          }
     }
};

I code this in PHP, it should be applicable in C++ but I don't know...

[o3hC5Di1]

Hi,

Thanks - but I think you misunderstood the question (which may be entirely my fault).
I wasn't asking about the code, I was asking about the design, the code is more or less pseudocode to make my situation clear.

Also, read() doesn't have to return anything if "u" is both an inbound and outbound parameter - which saves for the copying of the user object out of the function I believe.

All the best,
NwN

[o3hC5Di1]

Terribly sorry to bump this topic, but I'm still a little stuck on this. I'll try to summarize the problem a little bit briefer than in my original post.

I have a class base_user. I also have a class which maps (inserts, selects, updates) a base_user to the datastore. Now, when I create a derived_user, with additional datamembers I need to also create a derived class of the mapper class. The question is: how can I avoid this, for example with an xml file as datastore:

derived_user_mapper::read(derived_user& u)
{
    base_user_mapper::read(u);  //this opens the file, reads, closes the file
    std::ifstream f("users.xml"); //now I need to re-open it again to fetch the additional members 
    //read from file, close the file again
}

I have been thinking about passing lambda's into the base_user_mapper functions, or a signal/slots system, but I'm not sure what the best way for implementing those would be.

Any advice at all would be greatly appreciated - I'm a little stuck on my project because I don't feel like making a bad design decision that's going to cost me a lot of time to refactor.

All the best,
NwN

[coder777]

I'd say that you need to make the reader a member variable.
Then open it before reading (if it's not already open) and close it when it's not longer needed. That's maybe something for constructor/destructor if done cleverly (e.g. smart pointer)

[o3hC5Di1]

That's an interesting thought, but wouldn't that increase coupling between my user and user_mapper classes?
My goal - as this is for a (personal) library is to be able to switch seamlessly between, say, an xml mapper and sql mapper. It also didn't seem logical to me that a user-object would write or update itself to the datastore, because that would again increase coupling between what defines a user and the choice of datastore. Thanks for your input though, I will definitely look into it.

Any more thoughts?

All the best,
NwN

[coder777]

That's an interesting thought, but wouldn't that increase coupling between my user and user_mapper classes?

I meant make the reader (like ifstream) a member variable of the mapper class, like so:

struct base_user_mapper
{
   std::ifstream m_File;

    void open()
    {
        if(m_File.good())
          ;
        else
          m_File.open(...);
    }
    void close()
    {
        m_File.close();
    }
...
};

struct derived_user_mapper : public base_user_mapper
{
    void read(my_user& u)
    {
        open();
        base_user_mapper::read(u);
        m_File >> ...
        close();
        //etc.
    }

    void create(my_user& u)
    {
        open();
        base_user_mapper::create(u);
        ...
        close();
    }
};

[o3hC5Di1]

Ah, I see - that's already the case. The problem is that base_user_mapper actually maps base_user's, so to take your example:

struct base_user_mapper
{
   std::ifstream m_File;

    void open()  //open and close() are inherited from a mixin dependent on the datastore type
    {
        if(m_File.good())
          ;
        else
          m_File.open(...);
    }
    void close()
    {
        m_File.close();
    }

    void read(base_user& u)
    {
         m_File.open();
         m_File << u.id << u.name << u.password;
         m_File.close();
    }
};

struct derived_user_mapper : public base_user_mapper
{
    void read(my_user& u)  //so these would again be doing open/close
    { 
        open();
        base_user_mapper::read(u);
        m_File >> ...
        close();
        //etc.
    }

    void create(my_user& u)
    {
        open();
        base_user_mapper::create(u);
        ...
        close();
    }
};

To clarify, this is the structure of my classes:

base_user
|__ backend_user
|__ frontend_user


base_user_mapper 
|__ base_user_sql_mapper (inherits sql_mixin, maps base_user members to database)
|     |__ backend_user_sql_mapper
|     |__ frontend_user_sql_mapper
|
|__ base_user_xml_mapper (inherits xml_mixin, maps base_user members to xml-file)
      |__ backend_user_xml_mapper
      |__ frontend_user_xml_mapper

Sorry if I'm overcomplicating things - as I said, I've been stuck on this a little while so I might not be seeing the forest for the trees anymore.

Thanks for your input.

Edit: I'm starting to realize that the problem is actually that the base_user_<>_mapper's do the mapping for the base user themselves. If they contained the code for it (for instance, the select query), but would leave the actual execution of it up to the derived class, it should solve the issue. I would just have to make sure that the intent is made clear enough to avoid confusion. I'll see where this new perspective takes me, thanks a lot for thinking along and taking the time to respond coder777.

All the best,
NwN

[coder777]

well, in that scenario you must not use m_File.open() or m_File.close() directly. Using open() and close() only. You need a kind of reference count:

struct base_user_mapper_interface
{
    int m_OpenCount;
    base_user_mapper_interface() : m_OpenCount(0) { }

    void open()
    {
        if(m_OpenCount > 0)
          ;
        else
          imple_open();
        ++m_OpenCount;
    }
    void close()
    {
        --m_OpenCount;
        if(m_OpenCount > 0)
          ;
        else
          imple_close();
    }

    virtual void imple_open();
    virtual void imple_close();

    virtual void read(base_user& u)=0;
    virtual void create(base_user& u)=0;
    //etc.
};

struct base_user_mapper
{
   std::ifstream m_File;

    void imple_open()
    {
        if(m_File.good())
          ;
        else
          m_File.open(...);
    }
    void imple_close()
    {
        m_File.close();
    }

    void read(base_user& u)
    {
         open();
         m_File << u.id << u.name << u.password;
         close();
    }
};

struct derived_user_mapper : public base_user_mapper
{
    void read(my_user& u)  //so these would again be doing open/close
    { 
        open();
        base_user_mapper::read(u);
        m_File >> ...
        close();
        //etc.
    }

    void create(my_user& u)
    {
        open();
        base_user_mapper::create(u);
        ...
        close();
    }
};

Two more option:
1. You may also open it once and leave close to the destructor
2. You leave open and close entirely to the user of your mapping class.

[o3hC5Di1]

That seems like a great solution. I would have to ensure the reference count is thread-safe, but I'm sure I can figure that out. I'll need to make mock-ups of all the suggestions you did and decide on the best choice - which is a great step forward from having no idea to go about this, so thank you very much for your input.

Something is telling me that my confusion is a symptom of bad overall design, so I think it wouldn't hurt me to reconsider some of the existing classes too. I'll let you know which solution I come up with.

Thanks once more for your kind help.

All the best,
NwN