How to avoid code duplication with similar Django model methods?

Question

The following model contains the two almost identical functions list_ancestors and list_descendants. What would be a good way to write this code only once?

class Node(models.Model):
    name = models.CharField(max_length=120, blank=True, null=True)
    parents = models.ManyToManyField('self', blank=True, symmetrical=False)

    def list_parents(self):
        return self.parents.all()

    def list_children(self):
        return Node.objects.filter(parents=self.id)

    def list_ancestors(self):
        parents = self.list_parents()
        ancestors = set(parents)
        for p in parents:
            ancestors |= set(p.list_ancestors())  # set union
        return list(ancestors)

    def list_descendants(self):
        children = self.list_children()
        descendants = set(children)
        for c in children:
            descendants |= set(c.list_descendants())  # set union
        return list(descendants)

    def __str__(self):
        return self.name

EDIT: The solution derived from the answers below:

def list_withindirect(self, arg):
    direct = getattr(self, arg)()
    withindirect = set(direct)
    for d in direct:
         withindirect |= set(d.list_withindirect(arg))
    return list(withindirect)

def list_ancestors(self):
     return self.list_withindirect('list_parents')

def list_descendants(self):
     return self.list_withindirect('list_children')

@Sayse: I don't understand your question. Ancestors are a generalization of parents, so list_ancestors uses list_parents. Descendants are a generalization of children, so list_descendants uses list_children. — Watchduck
– Watchduck, Commented Jan 18, 2017 at 11:22
list_ancestors first gets the parents, and then recursively gets the ancestors of the parents. list_descendants first gets the children, and then recursively gets the descendants of the children. As these two similar functions I try to unify are recursive, my "solution" list_withindirect is recursive as well. — Watchduck
– Watchduck, Commented Jan 18, 2017 at 11:47

2ps · Accepted Answer · 2017-01-18 19:36:05Z

2

Use a string and call getattr on the object to get the callable function.

def list_withindirect(self, fn1):
    direct = getattr(self, fn1)()
    withindirect = set(direct)
    for d in direct:
         withindirect |= set(d.list_withindirect(fn1))

    return list(withindirect)

def list_ancestors(self):
     return self.list_withindirect('list_parents')

edited Jan 18, 2017 at 19:36

answered Jan 18, 2017 at 2:43

2ps

16k2 gold badges34 silver badges53 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Watchduck Over a year ago

That does not work. When I try to use list_ancestors, I get some ImportError and TypeError: list_withindirect() takes 2 positional arguments but 3 were given. I have no idea what you are trying to do with getattr BTW.

user2390182 Over a year ago

@Watchduck remove the second self in the last line

Watchduck Over a year ago

That did it. Thanks!

2ps Over a year ago

@schwobaseggl: (faceplam). Thank you for catching that.

Nikolay Prokopyev · Accepted Answer · 2017-01-18 13:19:38Z

0

This looks like the issue in bound and unbound methods problem.

When you're initially pass self.list_parents to self.list_withindirect(list_direct) everything is OK.

But when you're recursively pass the same! self.list_parents to d.list_withindirect (i.e. to descendants), you're accidentally populate your direct variable with parents of the topmost caller object, instead of d.

For example, it may be resolved using getattr, like it was answered by 2ps (upd: the error in his original code was found in comments there).

edited Jan 18, 2017 at 13:19

answered Jan 18, 2017 at 13:06

Nikolay Prokopyev

1,33213 silver badges23 bronze badges

2 Comments

Watchduck Over a year ago

d.list_direct leads to AttributeError: 'Node' object has no attribute 'list_direct'.

Nikolay Prokopyev Over a year ago

Sure. So, I wouldn't try to find wo getaatr solution (I'm sure it's possible), since you've already got an answer. I'm leaving only the reasons explanation of the original issue in my post.

Collectives™ on Stack Overflow

How to avoid code duplication with similar Django model methods?

2 Answers 2

4 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related