Ivan
Ivan

Reputation: 1455

Loop in functions calling each other

I have a class that represents a person in a family (FamilyMember).

I implemented the function setParent that receives a FamilyMember Object and adds it to the list of parent of the current Object. Exactly the same is done in the setChild function.

The problem: as one function calls the other (If we set the parent of our current Object we should as well set the current Object as the parent's child), they enter in a loop. => I managed to stop it checking if the object was already in the list. But this means I'm alway doing an unnecessary call to the function.

    def setParent(self, parent):
        if not isinstance(parent, FamilyMember):
            raise TypeError("Parent must be set to a FamilyMember")
        if parent in self.parents:
            print("parent already in the list")
            return
        self.parents.append(parent) 
        parent.setChild(self)


    def setChild(self, child):

        if not isinstance(child, FamilyMember):
            raise TypeError("child must be set to a FamilyMember")
        if child in self.children:
            print("child already in the list")
            return
        self.children.append(child) 
        child.setParent(self)

Posible solution that I dislike: I could do 2 more functions that do the same but don't call the other function. But this means repeating code, and having 4 different functions, which is not a good practice. ex:

    def setParent_no_loop(self, parent):
        if not isinstance(parent, FamilyMember):
            raise TypeError("Parent must be set to a FamilyMember")
        if parent in self.parents:
            print("parent already in the list")
            return
        self.parents.append(parent) 

    def setChild(self, child):
        if not isinstance(child, FamilyMember):
            raise TypeError("child must be set to a FamilyMember")
        if child in self.children:
            print("child already in the list")
            return
        self.children.append(child) 
        self.setParent_no_loop(self)

Posible solution that I dislike: I rewrite the code from the other function into both functions. Which, again is rewriting code, and its not really elegant:

    def setParent_no_loop(self, parent):
        if not isinstance(parent, FamilyMember):
            raise TypeError("Parent must be set to a FamilyMember")
        if parent in self.parents:
            print("parent already in the list")
            return
        self.parents.append(parent)
        parent.children.append(self) 

The question is, can you find an elegant way to solve this problem?

Upvotes: 0

Views: 223

Answers (3)

wwii
wwii

Reputation: 23783

Make parents and children sets instead of lists.

Then in each method just add and don't worry about it.

def setParent(self, parent):
    if not isinstance(parent, FamilyMember):
        raise TypeError("Parent must be set to a FamilyMember")
    self.parents.add(parent) 
    parent.children.add(self)


def setChild(self, child):
    if not isinstance(child, FamilyMember):
        raise TypeError("child must be set to a FamilyMember")
    self.children.add(child) 
    child.parents.add(self)

Upvotes: 0

Ben Taylor
Ben Taylor

Reputation: 515

You could hash the objects and check if they exist in the other via the hash comparison at the start of the set function? I've done it that way below (because that's safest if the user forgets to do the comparison first), but you could also do it before you call the set function to avoid the extra function call.

class FamilyMember(object):
    def __init__(self):
        self.relations = set()
        self.hashval = hash(self)
        self.parents = set()
        self.children = set()

    def knowsAbout(self, otherhash):
        if otherhash in self.relations:
            return True
        else:
            return False

    def setParent(self, newParent):
        if not self.knowsAbout(newParent):
            self.relations.add(newParent.hashval)
            self.parents.add(newParent)
            newParent.addChild(self)

    def setChild(self, newChild):
        if not self.knowsAbout(newChild):
            self.relations.add(newChild.hashval)
            self.children.add(newChild)
            newChild.addParent(self)

It's still an extra function call, but the hash comparison is a quick one at least.

Upvotes: 0

kaya3
kaya3

Reputation: 51152

I would suggest designing the methods as follows:

def add_child(self, child):
    self.children.append(child)
    child.parents.append(self)

def add_parent(self, parent):
    parent.add_child(self)

There is no need for these methods to be recursive or mutually-recursive. Add your own error-handling code as necessary.

Upvotes: 1

Related Questions