Map of derived functions

Question

I have Base and Derived class Base :

class Person{
public:
    Person(string name , int age ){
        this -> name = name;
        this -> age  = age;
    }
    virtual void getInfo(){
        cout << "Person " << name << " " << age;
    }

protected:
    string name;
    int age;
};

Derived:

class Kid : public Person{
public:
    Kid(string name, int age):Person(name,age){};
    virtual void getInfo( ){
        cout << "Kid " << name << " " << age;
    }
};
 class Adult : public Person{
    public:
        Adult(string name, int age):Person(name,age){};
        virtual void getInfo( ){
            cout << "Adult " << name << " " << age;
        }
    };

When i do something like

map<string ,Person*> m;
Person *one;
Person *three;
Kid two("Mathew",15);
Adult four("JOhn",55);
three = &four;
one = &two;
m["first"] = one;
m["second"] = three;
for( auto &x : m )
   x.second-> getInfo();
return 0;

It nicely prints info as it should // "Adult" for adult class and "Kid" for kid class

however when i edit the class and move the map into base class. e.g and create Add method.

 class Person{
    public:
        Person(string name , int age ){
            this -> name = name;
            this -> age  = age;
        }
        virtual void getInfo(){
            cout << "Person " << name << " " << age;
        }
        void add( string name , Person a){
            Person *one = &a;
            m[ name ] = one;
        }
        void print(){
            for( auto &x: m )
                 x.second -> getInfo()
        }
    protected:
        string name;
        int age;
       map< string , Person*> m;
    };



   Person one("John", 25);
   one.add("first",Kid("Mathew",15));
   one.add("second",Adult("Leo",55));
   one.print();

It throws seg fault , why is this happening? Its basicly the same implementation with using of method. What causes the seg fault? Is there a way how to fix it?

// EDIT

I tried using unique_ptr redecaring map as

map< string ,  unique_ptr<Person>> m;
   AddField (string name , Person   a ){

            m[name] = ( unique_ptr<Person> (a)); 
            return *this;
        }

or

  properties[name] = unique_ptr<Person> ( new Person( a ));

or

 AddField (string name , Person   a ){

        CData *one = unique_ptr<Person>(new Person(a));
        m[name] = one ;
        return *this;
    }

I am not experienced with unique/share ptr. This threw

‘std::unique_ptr’ to ‘Person*’

`Person *one = &a;` takes the address of a temporary copy of `Person`, pass a reference into the function. — πάντα ῥεῖ, Apr 30 '16 at 13:08
You could also consider having your map hold a shared_ptr or unique_ptr and then you have no issue with needing to delete. — Paul Rooney, Apr 30 '16 at 13:15
@PaulRooney I already [mentioned](http://stackoverflow.com/questions/36954159/map-of-derived-classes#comment61466426_36954159) that to OP. But, they seemingly cannot get it working (for whatever reason). — πάντα ῥεῖ, Apr 30 '16 at 14:09
I updated question for my attempt to create it with unique_ptr... — Darlyn, Apr 30 '16 at 15:16

Vittorio Romeo · Accepted Answer · 2016-04-30T13:30:44.863

Firstly, let's understand what the map is actually storing:

map< string , Person*> m;

This map binds a string to a Person*, which is a memory address pointing to a person. The map does not actually store the person instance, only its memory address.

Now let's analyze your two situations.

 map<string ,Person*> m;

// Allocating space for two memory addresses on the stack:
Person *one;
Person *three;

// Creating two instances on the stack:
Kid two("Mathew",15);
Adult four("JOhn",55);

// Setting the pointers to the memory addresses of the instances on the stack:
three = &four;
one = &two;

m["first"] = one;
m["second"] = three;
for( auto &x : m )
   x.second-> getInfo();
return 0;

// End of the function: now all objects are destroyed in reverse order.

The instances two and four live on the stack and are destroyed at the end of the scope (after return 0;). Taking their memory addresses and storing them into one and three is fine, as the pointers will outlive two and four.

Person one("John", 25);

// Creating a Kid instance without a name (temporary).
// The Kid instance goes out of scope immediately and is destroyed:
one.add("first",Kid("Mathew",15));

// Creating an Adult instance without a name (temporary).
// The Adult instance goes out of scope immediately and is destroyed:
one.add("second",Adult("Leo",55));

one.print();

The issue here is that the instances you are creating are destroyed too soon. You need to manage their lifetime properly to ensure that the memory address you're inserting into the map does not outlive the data in the pointed memory location.

Another major issue is that you're accepting add's Person a parameter by value. This will create a copy of the passed instance.

// Copy the passed instance:
void add( string name , Person a){
        // Take memory address of the copied instance:
        Person *one = &a;
        m[ name ] = one;

        // The copied instance is destroyed here!
    }

You should take a as a reference, in order to avoid copies and object slicing:

void add( string name , Person& a){
        m[ name ] = &a;
    }

Here's a working example, after correcting a's signature:

Kid k("Mathew",15);
Adult a("Leo",55);

// k and a will outlive one.

Person one("John", 25);
one.add("first", k);
one.add("second", a);

one.print();

// one gets destroyed.
// a gets destroyed.
// k gets destroyed.

A non const reference would not be able to bind to the rvalue he's passing in when he calls add. — Paul Rooney, Apr 30 '16 at 13:29
That's a good thing, as he's storing raw pointers inside the map. If he's not using any kind of smart pointer, he needs to make sure that the passed objects will outlive the map. A temporary would never outlive the map. — Vittorio Romeo, Apr 30 '16 at 13:32
Absolutely. I agree. As long as the op is clear that he has to change how he calls the function. I see you added that so all good. — Paul Rooney, Apr 30 '16 at 13:35
i would like to create a function which will does that as i previously wrote method add() , taking reference to object as argument does not work. — Darlyn, Apr 30 '16 at 13:56
@trolkura you can't pass by value. You'll get [object slicing](https://en.m.wikipedia.org/wiki/Object_slicing). It has to be pointer or reference. You should be looking at a unique or shared pointer. Depending on the needs of your program. — Paul Rooney, May 01 '16 at 04:26

score 1 · Answer 2 · answered May 02 '16 at 02:19

I had a go at trying to solve your issue and I came up an initial version of this. Which I'm not really happy with.

#include<iostream>
#include<map>
#include<memory>
#include<string>

/* used to simplify example code (not a good 
   idea in production code especially header files)
*/
using namespace std;

class Person
{
    public:
    virtual void getInfo() const = 0;
    virtual ~Person()
    {};

    void add(std::shared_ptr<Person> a)
    {
        m_[a->name_] = a;
    }

    void print() const
    {
        getInfo();
        for( auto &x: m_ )
             x.second->getInfo();
    }

    protected:
    Person(const string& name, int age)
        : name_(name), age_(age)
    {}    

    string name_;
    int age_;
    map<string , std::shared_ptr<Person>> m_;
};

class Kid : public Person
{
    public:
    Kid(const string& name, int age)
        : Person(name, age)
    {};

    virtual void getInfo() const override
    {
        cout << "Kid " << name_ << " " << age_ << '\n';
    }
};

class Adult : public Person
{
    public:
    Adult(const string& name, int age)
        : Person(name, age)
    {};

    virtual void getInfo() const override
    {
        cout << "Adult " << name_ << " " << age_ << '\n';
    }
};


int main()
{
    auto a = Adult("steve", 35);

    auto k1 = make_shared<Kid>("ben", 7);
    auto k2 = make_shared<Kid>("emily", 12);

    a.add(k1);
    a.add(k2);

    a.print();

}

I used shared_ptr as I'm guessing later you might want to retrieve those Persons from the map and return them from a getter call. So in that case unique_ptr makes no sense.

I think this version puts too much burden on the caller to create the shared_ptrs. Without knowing exactly what you plan to do though its difficult to suggest an alternative.

Map of derived functions

2 Answers2