Copying derived class without allocating memory?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
struct Object
{
    Object(int type):type(type){}

    virtual Object* clone()const = 0;

    int type;
};

class OReal : public Object
{
    public:
        OReal():Object(0),location(DIRT),kill(UNKNOWN),alive(true){}
        
        virtual OReal* clone()const{return new OReal(*this);}

        Location location;
        Kill kill;
        bool alive;
};

typedef std::unordered_map<std::string,std::unique_ptr<Object>> HASH_TABLE;

class State
{
    public:
        State(){}

        void addObject(const std::string& name,Object* obj){objects.insert(std::make_pair(name,std::unique_ptr<Object>(obj)) );} 
        //looks a little sloppy
        Object* getObject(const std::string& name){auto it = objects.find(name);return (it==objects.end()) ? nullptr : it->second.get();}
        const Object* getObject(const std::string& name)const{auto it = objects.find(name);return (it==objects.end()) ? nullptr : it->second.get();}

        State(const State& s)
        {
            for(const auto& p : s.objects)
                objects.insert(std::make_pair(p.first,std::unique_ptr<Object>(p.second->clone()) ));
        }
        State& operator=(const State& s)
        {
            assert(objects.size()==s.objects.size() && this!=&s);
            for(const auto& p : s.objects)
            {
                auto it = objects.find(p.first);
                assert(it!=objects.end());
                
                it->second = std::unique_ptr<Object>(p.second->clone());
            }
        }

        HASH_TABLE::iterator begin(){return objects.begin();}
        HASH_TABLE::iterator end(){return objects.end();}
        
    protected:
    private:
        HASH_TABLE objects;
};

int main()
{
    State state = existingState; //no way to avoid allocation

    //manipulate state

    state = existingState; //better to copy existing data without allocating
}


Dynamically allocating memory is not efficient when assigning one State to another. Is there a better way to copy one state to another when they are the same size with the same types of derived Objects?
Last edited on
Is there anything besides your intuition leading you to believe this is a problem?

Is there a good reason for a type named State to manage the lifetime of other objects?

If this actually is a problem, using custom memory pools might be a better idea than trying to salvage objects that may or may not be compatible.
I found a solution that at least compiles. The code looks a bit sloppy to me. Is there a cleaner way to do this?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
struct Object
{
    Object(int type):type(type){}
    Object(){}

    virtual Object* clone()const = 0;
    virtual bool copyTo(Object* obj) const= 0;

    int type;
};

class OReal : public Object
{
    public:
        OReal():Object(0),location(DIRT),kill(UNKNOWN),alive(true){}
        
        virtual OReal* clone()const{return new OReal(*this);}
        virtual bool copyTo(Object *obj)const
        {
            OReal* rObj = dynamic_cast<OReal*>(obj);
            if(rObj)
            {
                *rObj = *this;
                return true;
            }
            return false;
        }

        Location location;
        Kill kill;
        bool alive;
};

typedef std::unordered_map<std::string,std::unique_ptr<Object>> HASH_TABLE;

class State
{
    public:
        State(){}

        void addObject(const std::string& name,Object* obj){objects.insert(std::make_pair(name,std::unique_ptr<Object>(obj)) );} 
        //looks a little sloppy
        Object* getObject(const std::string& name){auto it = objects.find(name);return (it==objects.end()) ? nullptr : it->second.get();}
        const Object* getObject(const std::string& name)const{auto it = objects.find(name);return (it==objects.end()) ? nullptr : it->second.get();}
        
        int size()const{return objects.size();}

        State(const State& s)
        {
            for(const auto& p : s.objects)
                objects.insert(std::make_pair(p.first,std::unique_ptr<Object>(p.second->clone()) ));
        }
        State(State&& s)
        {
            objects = std::move(s.objects);
        }
        State& operator=(const State& s) = delete;
        
        bool copyTo(State& s)const
        {
            if(objects.size()!=s.size() || this==&s) return false;
            for(const auto& p : objects)
            {
                Object* obj = s.getObject(p.first);
                if(!obj) return false; //cant find match
                
                if(!p.second->copyTo(obj)) return false; //wrong type
            }
            return true;
        }

        HASH_TABLE::iterator begin(){return objects.begin();}
        HASH_TABLE::iterator end(){return objects.end();}
        
    protected:
    private:
        HASH_TABLE objects;
};
The code looks a bit sloppy to me. Is there a cleaner way to do this?

There sure is.

1
2
3
4
    void copyTo(State& s) const    // Why is this object modifying another object?
    {
        s.objects = objects;
    }


If the function returns the copy was a success and we haven't left some object in who-knows-what state. Of course, it might look a little better as:

1
2
3
4
5
    State& operator=(State s)
    {
        objects = std::move(s.objects);
        return *this;
    }


Thanks for answering the earlier questions. It sheds a lot of light on your design.
The first example you gave does not compile because unique_ptr cannot be assigned to an lvalue during the unordered_map copy. The second example works, but does not achieve good performance since the copy constructor will allocate new data for s when passed a lvalue. I am using copyTo to copy data without dynamically allocating data. One of my algorithms has a lot of state copying which is why I need it to only allocated data on the creation of a State instance and not every time an instance is set to another.
I am using copyTo to copy data without dynamically allocating data.

It looks like you'd mostly be using copyTo to partially copy an object and fail most of the time. Worse, you have no way of knowing what was successfully copied and what was not.

Again, if this is an issue for you (and it probably isn't) I would suggest the use of custom memory pools or a redesign of the class rather than a kludge of the sort you've presented.
The purpose of this entire class is to provide data storage for user defined data. A State instance is passed to user defined functions so that the user can retrieve his or her data by casting the pointers in the State to the appropriate derived class. There is not a way that I know of bypassing the necessity of using a container of pointers to user data to achieve this. To add to the complexity, some of my algorithms need to manipulate a state, perform some calculations, then reset the state back to the way it was before the computation, creating the necessity for efficient state copying. The format of the state should never change, so the copyTo function should never fail in any of my algorithms. The implementation of the State class will always require pointers of base classes and deep copying, no way around this. Also, I have no experience with custom memory pools. If you truly believe they are a good solution, then please elaborate on a design.
Finally, context.

The purpose of this entire class is to provide data storage for user defined data.

Judging from the following explanation, that is not it's purpose. Any container would fulfill that purpose.

You might want to check out the memento pattern.
https://en.wikipedia.org/wiki/Memento_pattern


Also, I have no experience with custom memory pools.

http://www.drdobbs.com/cpp/improving-performance-with-custom-pool-a/184406243
http://www.boost.org/doc/libs/1_60_0/libs/pool/doc/html/index.html
But, I wouldn't go this route unless profiling finds that allocation is a bottle neck. On the other hand, State would definitely benefit from a redesign.


Topic archived. No new replies allowed.