0

I am trying to create class Person with pointers to spouse(Person object) and table of children(Person objects). This class implements marriage(operator+=), divorce(method) and creating new children(operator++):

class Person{
private:
    char* name;
    int sex;
    Person* spouse; 
    Person* children[5];
public:
    Person();
    Person(const Person&);      
    Person & operator =(const Person&);
    Person & operator +=(Person&);//marriage
    Person & operator ++();//new children
    void divorce();//divorce
    Person::~Person();
}

I created destructor which deletes children whenever there is no spouse:

Person::~Person(){
    if (name !=NULL)
        delete [] name;
    name=NULL;

    if (spouse!=NULL)
        spouse->spouse =NULL;
    else{
        for (int i=0; i<5;i++){
            if (children[i]!=NULL)
                delete children[i];
            children[i]=NULL;
            }
    }
}

I do not know if my copy constructor and operator= should create another instances of spouse and children. I tried to do this but I was stack in infinite reference. Is it possible to create a copy of Person object with properly assigned spouse and children?

Thanks in advance for any comments and suggestions Elkhunter

elkHunter
  • 55
  • 2
  • 9
  • 3
    Completely offtopic - but you do know *children* is already plural of *child*, and so *childrens* is just a grammatical error? Also, there are people in this world with more than 5 children, so I'd recommend putting it in an `std::vector` instead. Also solves the deletion issue with them. – Niels Keurentjes Jan 19 '14 at 17:16
  • Why use a pointer for `name` and not [`std::string`](http://en.cppreference.com/w/cpp/string/basic_string)? Also, you are properly initializing the `childrens` and `spouse` members in the constructor? And what if a person have more than five children? Maybe use e.g. [`std::vector`](http://en.cppreference.com/w/cpp/container/vector) instead? – Some programmer dude Jan 19 '14 at 17:19
  • @NielsKeurentjes: I tend not to worry too much about grammar in symbol names. Consider a class `Person`: if you instantiate it twice, you have two `Persons`. This is actually valid in English in edge cases, but you see what I mean. If you need a better example, what the heck is a "destructor"?! That said, he _did_ mean "children" here :) – Lightness Races in Orbit Jan 19 '14 at 17:24
  • Sorry for childrens. Of course std::string will be more easier to handle and vector or dynamic array more appropriate for children... but this is an exercise from my studies and I do not want to change any data types. – elkHunter Jan 19 '14 at 17:30

2 Answers2

2

I think that your designing it wrong way. Class Person should have preferably only one well defined responsibility. Right now, it's representing at least two separate concepts - a person and a family. Consider splitting those two concepts into separate types - it should make it easier to implement it.

tumdum
  • 1,981
  • 14
  • 19
  • Unfortunately, I cannot change any properties of this class. I think that my professor wants to check our knowledge about constructors, destructors, etc. – elkHunter Jan 19 '14 at 17:47
0

Your problem is that you're trying to recursively handle intertwined relationships. When a Husband has a Wife, and both delete the other, you're bound to get stuck in recursive hell of course.

Instead, keep the references as pointers, and do not delete any of them in the destructor - it's simply not the responsibility of the class. Imagine when a real life husband dies, does his wife also die automatically?

Just keep a separate std::vector of Person instances as the 'master catalogue' of people in your 'world', and let them reference eachother lazily through their relationships. Puts all the deletion misery in one extremely non-recursive place, and solves all issues.

Niels Keurentjes
  • 41,402
  • 9
  • 98
  • 136
  • I do not delete husband in my destructor. When wife dies I delete pointer to wife in husband class and do nothing with children. If wife has no husband I delete her children because no one else know about them (there is no more pointers to children objects). Still I have no idea how to create copy-constructor and operator=. – elkHunter Jan 19 '14 at 17:40
  • Hmm... 'master catalogue' can save me a lot of work. With this instance I could simplify my destructor and do not worry any longer about proper look of copy constructor. But does it mean that there is no other way of implementing correct constructor without 'master catalogue'? – elkHunter Jan 19 '14 at 17:53
  • The alternative to avoid mem leaks would be reference counting. – Niels Keurentjes Jan 19 '14 at 18:22
  • Thank you for a tip - I will read about it. I have one more question regarding 'master catalogue' idea. In my copy constructor I still need to create another spouse(husband) object. And than I stil get stuck with circular reference - when I copy husband he has wife and so on. I think I miss something in copy constructor idea. Could you tell me how can I avoid this endless loop with copying Person object? – elkHunter Jan 19 '14 at 18:35
  • As long as you don't walk any **pointers** recursively you'll be fine. Copying a pointer is no problem as long as the master instance keeps existing. You may want to read up on the concept of pointers in that regard. As mentioned, you could also implement a reference counted smart pointer on the people in your 'database'. – Niels Keurentjes Jan 19 '14 at 19:25
  • As far as I know I shouldn't copy a pointer in a copy-constructor. With pointer to Person I used the same methodology as with pointer to name array(char *). So I created another object and set the pointer to this new instance. And that generates my recursive hell. Right now I am thinking of setting husband and children to NULLS in copy-constructor and operator=. I will also throw an exception whenever someone tries to copy object which is married or has children. I hope it will work out. – elkHunter Jan 19 '14 at 20:35