Two pointers pointing to same memory via class copy constructor. How to detect when one deletes the object?

[helios]

Doing this purely in C++ is impractical (although less so if you accept a code generation step before build time), but it's actually possible to release general cycles using only reference counting. What you do is, whenever an object is about to be released, walk the graph of reachable objects and slice the graph into nodes only reachable from the node being deleted and nodes reachable from unrelated nodes. This can be done in O(n) time (over the size of the reachable graph) using only the reference counts and three extra fields that are attached to the object.
I wrote a PoC a few years ago: https://gist.github.com/Helios-vmg/382077403922655f215bc9526abb0883

[dhayden]

Yes, this is just an example. I'm trying to dust off my C++ skills after years and years of programming in other languages.

Unfortunately, you've picked a really nasty problem to play around with pointers. If you just want to mess around with the language, try implementing your own linked list or binary tree.

The problem with genealogical relationships is that they gets incredibly complex. Here are some problems with your code:
- when you set person1.addSpouse(person2), it should make each person a spouse of the other, not just one. Don't rely on the caller to keep this relationship.
- If you delete a person, you remove their children, but you don't remove the person from the parent's list of children.
- A child, by definition, has two parents. So when you add a child to one person, shouldn't you add them as a child to their other parent?

To handle all of this in a generic way, it would be better to assign each person an ID. A collection of Persons holds the people. Each person object has a collection of relationships:

class Relationship {
    RelationshipType type;   // type of relationship
    unsigned id;                  // id of the other party.
};

Now if you want to delete a person, then you remove them from the collection of people. Part of that process is to go through all the other people and remove any relationships to the one being deleted.

Like I said, practice with a linked list or binary tree. :)

[RickBlacker]

dhayden...

Yes, I fully agree. This wasn't an exercise is creating a realistic app. Real workable functionality of the class was mostly an afterthought.

I agree with you that a hierarchy such as this could get nasty real quick given the fact that kids could have kids, have cousins etc etc... Factor in divorce, people dying etc. :)

The goal here was to create a basic class that had constructors, copy constructor, operator overloading. Used pointers. I wanted to be sure that I had the basics of these concepts working properly. Was I allocating memory properly, did I create any memory leaks. Etc...

The business logic errors

AddSpouse( Person* p). Yep, big hole here... You can't associate the two in the function, if you do it creates a recursion problem. Agreed that there needs to be a better association design between the two married people.

Deleting a person... Good catch. Didn't really think of it. But, again... I wasn't too worried about the business logic at the time. Yes, this is a hole in the implementation.

Adding the child.... Yes, I had originally done something like that, but then I thought about the situation where, what if two people came into a marriage and had existing kids. He had his, she had hers. So, I kept the child vectors in tact, then if I need to know ALL the combined kids, I just search the Person's vector AND the spouses vector. This can be seen in the ToString() function. Not saying it's the most accurate way to do it, but it served my purpose for "THIS" exercise.

I do like the idea of playing with data structures. I have not implemented a data structure in C++ since my college days back in the mid 90s. I used to have a really good book in C++ data structures that had many common data structures in them. But, I loaned that book out and never got it back... OH well. I'm sure I can find the same info on the internet now.

[Cubbi]

RickBlacker wrote:

The goal here was to create a basic class that had constructors, copy constructor, operator overloading. Used pointers.

This isn't 1990's anymore. A well-written C++ class today will not define the copy constructor/assignment operator/destructor. A C++ program written today will never use "new" or "delete" and will have almost no pointers. Certainly never a pointer that can be deleted.

Consider maintaining all people in a single static hash table: std::unordered_map<SSN, Person>.
Make the Person constructor private (and get rid of copy/assignment/destuctor)
Give your Person these private members:

    std::string name;
    std::array<SSN, 2> parents;
    std::optional<SSN> spouse;
    std::vector<SSN> kids;

and this public interface:

// non-members:
// get the next available SSN, update parents if known, make a new entry in the hash table, return a reference to the person
Person& make_person(Person& parent1 = Unknown, Person& parent2 = Unknown);
// update the spouse SSNs in both Persons, or return false if either is already married
bool marry(Person&, Person&); 
// replacement for your ToString: no need to allocate strings if all you need is output
std::ostream& operator<<(std::ostream&, const Person&) 

[RickBlacker]

This isn't 1990's anymore. A well-written C++ class today will not define the copy constructor/assignment operator/destructor. A C++ program written today will never use "new" or "delete" and will have almost no pointers. Certainly never a pointer that can be deleted.

I'm curious to know why it is a more modern C++ app written today would not have some of these capabilities in them. I can understand suggesting people use smart pointers over standard pointers. Do you have any links to further reading C++ best practices?

I like the idea of overloading the << operator.

While I fully admit that my implementation is not ideal and was never meant to be a useful example. It raises a question about your suggestion of a hash table. If I were to try to make a real world implementation of this, I would have perhaps thought about using a Tree. Having said that... I have not done ANYTHING with a tree data structure since college and I admit, i could be opening Pandora's box by trying to use one. In fact, using a tree may not even be a wise choice. I'd really need to go back and read up on this data structure. If memory serves me right, there are different types of trees that can be used.

[keskiverto]

I'm curious to know why it is a more modern C++ app written today would not have some of these capabilities in them.

They have, but hidden. Mostly hidden by encapsulation that is known as C++ Standard Library.

Take the std::vector. It is essentially a dynamically allocated array, but we don't have to write new and delete [] if we use it, nor worry whether we do them properly. Somebody else has done the legwork for us.


Default constructor, copy constructor, move constructor, copy assignment, move assignment, destructor. Some of them the compiler generates for us.

If you have a pointer member, then the default copy is a shallow copy.
If that member is a shared_ptr, then the default is still "shallow", but we won't face double delete error.
If that member is a std::vector, then copies are deep.

In other words, if you do use existing "smart" types in your class, then there is less need to babysit (i.e. implement constructors).

[Cubbi]

RickBlacker wrote:

why it is a more modern C++ app written today would not have some of these capabilities in them

The capabilities are there, but made harder to use incorrectly -- the question that started this thread "Two pointers pointing to same memory via class copy constructor. How to detect when one deletes the object" is an example of incorrect usage of pointers.

RickBlacker wrote:

Do you have any links to further reading C++ best practices?

Certainly, there is a current project by Stroustrup and Sutter on documenting exactly that: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md

The things I mentioned are listed there as:
C.20: If you can avoid defining default operations, do
R.11: Avoid calling new and delete explicitly
R.3: A raw pointer (a T*) is non-owning