C++ core dump error while implementing family tree

user_9100626 0
Viewed 75

I'm trying to implement family tree. I have the classes Person and Tree defined as follows:

File FamilyTree.hpp:

using namespace std;
#include <string>

namespace family{

    class Person{
        public:
        string name;
        Person* mother;
        Person* father;

        Person(string name);

    };

    class Tree{
        public:
        Person* root;

        Tree(string name);

        Tree& addFather(string name1, string name2);
        Tree addMother(string name1, string name2);
        void display();
        string relation(string name);
        string find(string name);
        void remove(string name);
    };

};

File FamilyTree.cpp:

#include "FamilyTree.hpp"
#include <string>
#include <iostream>
using namespace family;   

// FUNCTIONS

Person& findPerson(Person& root, string child_name){
    if (root.name.compare(child_name) != 0)
    {
        cout<<root.name<<":1"<<endl;
        findPerson(*root.father, child_name);
    }
    else if(root.name.compare(child_name) == 0){
        cout<<root.name<<":2"<<endl;
        return root;  
    }else{
        cout<<"not found!!!"<<endl;
        Person p("no found");
        return p;
    }
}

// PERSON
family::Person::Person(string person_name){
    name = person_name;
    father = nullptr;
    mother = nullptr;
};

// TREE
family::Tree::Tree(string name){
    root = new Person(name);
};

family::Tree& Tree::addFather(string child, string father){
    Person& child_found = findPerson(*root, child);

    //cout<<"child_found.name:"<<child_found.name<<endl;
    child_found.father = new Person(father);
    return *this;
    };

family::Tree family::Tree::addMother(string name1, string name2){return Tree("");};
void family::Tree::display(){};
string family::Tree::relation(string name){return "";};
string family::Tree::find(string name){return "";};
void family::Tree::remove(string name){};

int main(){
    Tree t("X");

    t.addFather("X", "Y");
    t.addFather("Y","Z");
    return 0;
}

I started with addFather() function: addFather("child", "new father") for adding new father for existing child. I implemented it recursively using findPerson() function, which returns the Person object for the child and addFather() func creating new Person and initializing it to found child.

After I add 2 fathers, I get Illegal instruction (core dumped) error, what should be the problem?

core compiler-errors coredump c++
3 Answers

If you turn on your warnings, you'll see that you're not always returning from findPerson. In this function

Person& findPerson(Person& root, string child_name){
    if (root.name.compare(child_name) != 0)
    {
        cout<<root.name<<":1"<<endl;
        findPerson(*root.father, child_name);      // (1)
    }
    else if(root.name.compare(child_name) == 0){
        cout<<root.name<<":2"<<endl;
        return root;  
    }else{
        cout<<"not found!!!"<<endl;
        Person p("no found");
        return p;                      // (2)
    }
}

The code in the first if-branch, marked (1), needs to return the recursively-found Person, like this

return findPerson(*root.father, child_name);

That should take care of the segfault.

However, there is a deeper issue in the last branch of this function, where you are returning a reference to a local variable p, marked (2). If you do this, you are returning a dangling reference, since p will go out of scope when the function returns.

You need to think about what this function should do if no Person is found.

  • You could maybe return a Person*, so nullptr means no Person is found.

  • You could return an std::optional<Person>.

user_83728530

The problem is that you cannot return a reference to a local object. THis will result in undefined behavior.

When you add a new member, the findPerson() will not find it an try to return such a local object p. This is doomed to fail.

This design is not optimal. Possible solutions:

  • declare a static object p that is returned when nothing is found. Unlike locals, static remain alive and the reference returned would then stay valid. You have to make sure however that this the returned reference is never used in a way that may change the name of your static object.
  • change the interface of findPerson() so that it return pointers. Either it returns a valid pointer to a person, or it returns nullptr if nothing is found. This is a common idiom (in real world development, you would return an iterator but if you're beginning in C++, this would be too complex for the time being).
  • throw an exception if nothing is found and rewrite your calling code so that it catches the exception. I would not advise to use this approach here: exceptions are better kept for really exceptional situations.

Note that I did not look in your code if there were other errors. I just stoped at the first attempt to find something that doesn't exist when the tree is empty.

user_37234230

It working for fathers search, but I need it to iterate over all of the tree. When I added mothers root it core dumping again.

Person& findPerson(Person& root, string child_name){
    if (root.name.compare(child_name) != 0)
    {
        //cout<<root.name<<":1"<<endl;
        return findPerson(*root.father, child_name); 
        return findPerson(*root.mother, child_name);     // (1)
    }
    else if(root.name.compare(child_name) == 0){
        //cout<<root.name<<":2"<<endl;
        return root;  
    }else{
        throw exception();                     
    }
}
user_91006260
Related