0
\$\begingroup\$

Let's consider the following assumptions:

  • The Python programming language is used.
  • One composite class is implemented.
  • There are two or more component classes implemented. For instance, the composite class is Face which has two or more composite classes like Nose and Mouth.
  • The component classes have a single responsibility. For instance, considering the previous example, both classes compute geometrical features only, like mouth width, nose height, etc.
  • Lengths, widths, etc. are implemented as lazily computed and immutable properties (i.e. once a property is computed, it's cached and cannot be overwritten) by means of a @lazyproperty decorator (for reference see recipe 8.10 in the Python Cookbook, Third Edition or you see the code on GitHub.). This is because a user of the Face class may be interested in computing just one of the properties or a small subset and not all of them for 1,000 different faces. Moreover, logic can be added to the class without breaking the class interface.

Question: is the composite class, i.e. Face, defined in the code below properly composed? The code itself works, but I am wondering if the structure of the code is Pythonic or maybe it can be simplified/improved.

In the code below I use the pass statement just because the code of the properties is not relevant for this question. I am interested in the overall design of the composite class.

def lazyproperty(func):
    name = '_lazy_' + func.__name__
    @property
    def lazy(self):
        if hasattr(self, name):
            return getattr(self, name)
        else:
            value = func(self)
            setattr(self, name, value)
            return value
    return lazy


class Nose:
    def __init__(self, mesh):
        self._mesh = mesh


    @lazyproperty
    def height(self):
        # return nose height
        pass

    @lazyproperty
    def width(self):
        # return nose height
        pass


class Mouth:
    def __init__(self, mesh):
        self._mesh = mesh

    @lazyproperty
    def area(self):
        pass

    @lazyproperty
    def width(self):
        pass


class Face:
    def __init__(self, mesh):
        self._mesh = mesh

    @lazyproperty
    def nose(self):
        return Nose(self._mesh)

    @lazyproperty
    def mouth(self):
        return Mouth(self._mesh)
\$\endgroup\$
4
  • \$\begingroup\$ Could you maybe link the recipe from the cookbook in its entirety? In case you don't know, it's all hosted on github here github.com/dabeaz/python-cookbook. \$\endgroup\$
    – ades
    Jan 9 at 16:16
  • 1
    \$\begingroup\$ @ades thanks! I added the GitHub link. \$\endgroup\$
    – blunova
    Jan 9 at 16:20
  • \$\begingroup\$ Instead of your custom lazyproperty() you could use functools.cache() followed by property(). \$\endgroup\$ Jan 9 at 20:05
  • 2
    \$\begingroup\$ Welcome to Code Review blunova. Unfortunately your question is currently off-topic. Generic best practice questions cannot be reasonably answered. And we only review code from an author or maintainer of the code. Once you edit your post to fix the issues we'll be happy to review your code. \$\endgroup\$
    – Peilonrayz
    Jan 10 at 11:24
0
\$\begingroup\$

It's a little bit difficult to give feedback without having access to the book you reference, but I'll try nonetheless. To be fair, it looks like it's mostly the lazy property bit that's taken verbatim from the book, and it's not super relevant for your actual question anyway (feedback on usage of composition).

Are you using composition? Yeah, looks like it, and I don't see any issues. On that topic, however, the best way to avoid issues is if you write up a short test suite - and I cannot encourage doing so enough. Unit tests are your friends.

I would probably advise against just have pass implemented for non-abstract methods - you could raise NotImplementedError if you haven't yet implemented it. But I suppose it's possible that you also just left pass in there as basically an ellipsis (...) replacement, to indicate that that implementation doesn't matter for your question/example, which is fine.

Finally, coming back briefly to the lazy properties, I don't think they make much sense here. Calculating the a nose dimension doesn't seem very costly, so there's no reason to deal with this lazily.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks @ades! Unfortunately I cannot upvote your answer. I used pass just because it was not interesting to show the actual code for those properties. The question is about the overall design of the composite class. I edit the question to make that more explicit. You are right that maybe these properties are computationally not expensive. However, what about a user that has, let's say, 1,000 different meshes (i.e. faces), but it's interested in computing just one single property for each face? For this case having these properties lazy computed would make more sense to you? \$\endgroup\$
    – blunova
    Jan 9 at 16:49
  • \$\begingroup\$ Ok gotcha @blunova. I'd only really do that lazily if I knew I only needed to care about the Noses (or its dimensions, etc.) for a subset of the Faces. And I'd generally only start caring about it when I saw or foresaw a performance hit. Otherwise I'd mostly just have lazy evaluation if it's part of a longer chain of dynamic operations; if you have to form a huge map of Faces but you'll only care about select ones. If it's buffering you're after I'd use a cache. The use case I see for lazy properties is when you have both of those needs. \$\endgroup\$
    – ades
    Jan 9 at 16:59

Not the answer you're looking for? Browse other questions tagged or ask your own question.