Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

load_and_construct with class inheritance #237

Closed
kylefleming opened this issue Sep 30, 2015 · 5 comments
Closed

load_and_construct with class inheritance #237

kylefleming opened this issue Sep 30, 2015 · 5 comments

Comments

@kylefleming
Copy link
Contributor

@kylefleming kylefleming commented Sep 30, 2015

Hi,

I was wondering how you would handle load_and_construct with an inheritance hierarchy. Here's an example:

#include <iostream>
#include <memory>

#include <cereal/archives/json.hpp>
#include <cereal/types/memory.hpp>
#include <cereal/types/base_class.hpp>
#include <cereal/types/polymorphic.hpp>

using namespace std;

struct A {

    explicit A(int x_) : x(x_) {}
    virtual ~A() = default;

    int x;

    template<typename Archive>
    void serialize(Archive& archive)
    {
        archive(x, hidden);
    }

    template <class Archive>
    static void load_and_construct( Archive & ar, cereal::construct<A> & construct )
    {
        int x;
        ar(x);
        construct(x);
        // Issue #1
    }

private:

    bool hidden = false;

};

struct B : A {

    explicit B(int x, string y_) : A(x), y(y_) {}

    string y;

    template<typename Archive>
    void serialize(Archive& archive)
    {
        archive(cereal::base_class<A>(this), y);
    }

    template <class Archive>
    static void load_and_construct( Archive & ar, cereal::construct<B> & construct )
    {
        // Issue #2
        int x;
        string y;
        ar(x, y);
        construct(x, y);
    }

};

CEREAL_REGISTER_TYPE(B);

int main() {
    shared_ptr<B> b = make_shared<B>(10, "1");

    std::stringstream ss;
    {
        cereal::JSONOutputArchive oarchive(ss);
        oarchive(b);
    }
    cout << ss.str() << endl;
    shared_ptr<B> b2;
    {
        cereal::JSONInputArchive iarchive(ss);
        iarchive(b2);
    }
}

Two main questions I have around this, is:

  1. How do you handle serialized state that isn't restored directly from the constructor. I.e. if you wanted to restore hidden when deserializing A. There's not an object to act upon inside load_and_construct and since you already extracted some state from archive, it doesn't seem like calling serialize after construction is viable either.

  2. How would you handle inheritance when using load_and_construct? It doesn't make sense to call load_and_construct on the base class, because then you'd also be trying to reconstruct the object as if it was an instance of the base object (unless some template trickery is going on here as well, but I don't really see how it could be solved this way either). Calling up through the normal serialize flow seems like the lesser of the evils, but again it still has issues since you'll potentially be extracting some of the data twice from the archive.

@M2tM

This comment has been minimized.

Copy link

@M2tM M2tM commented Sep 30, 2015

Here's an example of some of my serialize and load_and_construct code which may implicitly answer your questions:

Base Class: Component

        template <class Archive>
        void serialize(Archive & archive) {
            archive(
                CEREAL_NVP(componentId),
                CEREAL_NVP(componentOwner)
            );
        }

        template <class Archive>
        static void load_and_construct(Archive & archive, cereal::construct<Component> &construct) {
            construct(std::shared_ptr<Node>());
            archive(
                cereal::make_nvp("componentId", construct->componentId),
                cereal::make_nvp("componentOwner", construct->componentOwner)
            );
            construct->initialize();
        }

Derived Class: Component::Drawable
...

        template <class Archive>
        void serialize(Archive & archive) {
            archive(
                CEREAL_NVP(shouldDraw),
                CEREAL_NVP(ourTexture),
                CEREAL_NVP(shaderProgramId),
                CEREAL_NVP(vertexIndices),
                CEREAL_NVP(localBounds),
                CEREAL_NVP(drawType),
                CEREAL_NVP(points),
                cereal::make_nvp("Component", cereal::base_class<Component>(this))
            );
        }

        template <class Archive>
        static void load_and_construct(Archive & archive, cereal::construct<Drawable> &construct) {
            construct(std::shared_ptr<Node>());
            archive(
                cereal::make_nvp("shouldDraw", construct->shouldDraw),
                cereal::make_nvp("ourTexture", construct->ourTexture),
                cereal::make_nvp("shaderProgramId", construct->shaderProgramId),
                cereal::make_nvp("vertexIndices", construct->vertexIndices),
                cereal::make_nvp("localBounds", construct->localBounds),
                cereal::make_nvp("drawType", construct->drawType),
                cereal::make_nvp("points", construct->points),
                cereal::make_nvp("Component", cereal::base_class<Component>(construct.ptr()))
            );
            construct->initialize();
        }

...

@kylefleming

This comment has been minimized.

Copy link
Contributor Author

@kylefleming kylefleming commented Sep 30, 2015

Ok, that makes a lot of sense. Thanks!

I still have some confusion about how I would construct a derived class that requires state from the base class in order to construct (the x variable in my example above). Since you need to extract x in order to construct, calling serialize on the base class will extract that data twice giving issues.

Maybe relying on NVP sidesteps that issue and you just let it extract and set that variable twice? Extraneous work though.

I can write up another example tomorrow if there's confusion about what i just described.

Side note: I think it would be worth putting a note in the docs about being able to dereference the construct object to get the recently constructed object (or maybe I missed it?). Also the technique of calling the base class's serialize from the load_and_construct wasn't immediately obvious. A note about that could also be helpful to future users.

@AzothAmmo

This comment has been minimized.

Copy link
Contributor

@AzothAmmo AzothAmmo commented Sep 30, 2015

A few comments on your example.

  1. If you have private members being serialized you need to befriend cereal::access
  2. You need to do serialization in the same order regardless of which function it takes place in

Your A load_and_construct function should look something like:

    template <class Archive>
    static void load_and_construct( Archive & ar, cereal::construct<A> & construct )
    {
        int x;
        bool h;
        ar(x, hidden);
        construct(x);
        construct->hidden = h; // pointer access to the object
    }

Note that the order in which things are retrieved from the archive matches that in your serialization code. Details on the -> operator are in the doxygen for cereal::construct, but probably not fully detailed in the more general website doc.


The situation for B is a bit more problematic given the restrictions I've outlined. I'm not sure if there is a clean way to do this right now.

@M2tM

This comment has been minimized.

Copy link

@M2tM M2tM commented Sep 30, 2015

Is something like this in line with what might work? Note I pass in a "junk" constructor value that the serialize method in the base will overwrite the value with anyway, so... Make sure if you have any post construction steps that would normally happen in the constructor you account for this in your serialize methods:

A:

        template<typename Archive>
        void serialize(Archive& archive)
        {
            archive(x, hidden);
        }

        template <class Archive>
        static void load_and_construct( Archive & ar, cereal::construct<A> & construct )
        {
            int x;
            ar(x);
            construct(x);
            ar(construct->hidden);
        }


    B:

        template<typename Archive>
        void serialize(Archive& archive)
        {
            archive(y, cereal::base_class<A>(this));
        }

        template <class Archive>
        static void load_and_construct( Archive & ar, cereal::construct<B> & construct )
        {
            string y;
            ar(y);
            construct(-1, y); //note junk -1 data!
            ar(cereal::base_class<A>(construct.ptr()));
        }

Now I haven't tested this, but let's assume construct(-1, y) does something with the -1 value and you only ever do it in construct... Maybe you have to do some kind of construction step and now that you don't have X on initialization it messes up your flow. I think this may work, I'm curious so let me know:

    A:

        template<typename Archive>
        void save(Archive& archive) const
        {
            archive(x, hidden);
        }

        template<typename Archive>
        void load(Archive& archive) //The *ONLY* way you can get here is through a derived class being deserialized.
        {
            archive(x, hidden);
            //Here we *know* we've been called through by load_and_construct.
            //We can safely perform the proper constructor operation here if you need to do something with x.
        }

        template <class Archive>
        static void load_and_construct( Archive & ar, cereal::construct<A> & construct )
        {
            int x;
            ar(x);
            construct(x);
            ar(construct->hidden);
        }
@kylefleming

This comment has been minimized.

Copy link
Contributor Author

@kylefleming kylefleming commented Oct 1, 2015

Thanks @AzothAmmo for the info.

@M2tM, interesting approach. I think at that point, the lesser of the evils is for me to rearchitecture the hierarchy to use default constructors and have 2-phase construction using a factory method or constructor functions or something. Not always possible, but in this case it is. (Mainly I think sending junk data to a constructor might be a recipe for errors down the line)

Thanks for the help :)

@kylefleming kylefleming closed this Oct 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.