CODEWITHSUNDEEP
c++design-patterns
Skamah One
Skamah One

Reputation: 2456

Two members being dependent on each other?

Imagine having a class called Node that can hold multiple parents and multiple children:

class Node {
    public:
        Node();
        virtual void addParent(Node *parent);
        virtual void addChild(Node *child);
    private:
        vector<Node*> m_parents;
        vector<Node*> m_children;
};

The problem is that every time you add a parent to a node, the node's m_parents must be updated and the parent's m_children must be updated; this creates an infinite loop.

void Node::addParent(Node *parent)
{
    if (m_parents.lacks(parent)) { // supposing such method exists
        m_parents.push_back(parent);
    }
    parent->addChild(this);
}

void Node::addChild(Node *child)
{
    if (m_children.lacks(child)) {
        m_children.push_back(child);
    }
    child->addParent(this);
}

As you can see, it's no good. I've managed to solve this by having four methods for adding instead of two, but it feels somewhat stupid. The additional two methods are both declared private or protected so they can't be called by others. Here's the original addParent and a new method called rawAddChild:

void Node::addParent(Node *parent)
{
    if (m_parents.lacks(parent)) {
        m_parents.push_back(parent);
    }
    parent->rawAddChild(this);
}

void Node::rawAddChild(Node *child)
{
    if (m_children.lacks(child)) {
        m_children.push_back(child);
    }
    // Doesn't call for parent's method
}

This would obviously be the same for addChild() and rawAddParent().

However, this doesn't feel like a proper solution, and it's certainly not clear for "outsiders" why there's addChild and rawAddChild methods. Is there a problem with my logic and if so, how should I solve such problem? Or is my solution already good?

Upvotes: 0

Views: 108

Answers (3)

dchhetri
dchhetri

Reputation: 7136

You should abstract a little.

bool Node::addParent(Node *parent)
{
    if (m_parents.lacks(parent)) { // supposing such method exists
        m_parents.push_back(parent);
        parent->updateRelationShip(parent,this);
        return true;
    }
    return false;
}

bool Node::addChild(Node *child)
{
    if (m_children.lacks(child)) {
        m_children.push_back(child);
        child->updateRelationShip(this,child); 
        return true;
    }
    return false;
}
void Node::updateRelationship(Node*parent, Node* child){
   parent->addChild(child);
   child->addParent(parent);
}

Upvotes: 0

Nicholaz
Nicholaz

Reputation: 1429

I would test if the vector already constains the element (probably using set instead of vector)

void Node::addParent(Node *parent)
{
    if (m_parents.lacks(parent)) { // supposing such method exists
        m_parents.push_back(parent);
    }
    if (parent->m_childs.find(this)==set::end)
        parent->addChild(this);
}

void Node::addChild(Node *child)
{
    if (m_children.lacks(child)) {
        m_children.push_back(child);
    }
    if (child->m_parents.find(this)==set::end)
        child->addParent(this);
}

Upvotes: 0

slaphappy
slaphappy

Reputation: 6999

I would suggest doing the actual operation in only one of the two methods:

void Node::addParent(Node *parent)
{
    if (m_parents.lacks(parent)) {
        m_parents.push_back(parent);
    }
    if (parent->m_children.lacks(this)) {
        parent->m_children.push_back(this);
    }
}

void Node::addChild(Node *child)
{
    child->addParent(this);
}

Upvotes: 7

Related Questions