2
\$\begingroup\$

I'm modelling the core entities in a callcenter-related system that deals with Operators and groups ("pools") of them. While I've written some Python before, this is my first time building a larger project in it from scratch, and I'd like to know if my approach and code is pythonic enough or will get me into trouble later on.

I really like the idea of interfaces in general and for explorational design specifically, and while Python doesn't have those, ABCs seem like the next best equivalent thing. It there anything to be said about my use of them?

Any specific hints about my use of a MutableSet?

I'm a bit worried about file structures and putting every concrete class in its own file, as that results in from Foo.Bar.Baz import Baz; is that acceptable, or is the common wisdom to include more classes in a single module?

I'm also using a lot of type hinting; while I'm aware that it doesn't actually do anything in Python, it does help me keep the ducks straight in my head while typing and helps my IDE help me. Any concerns here?

# core/abstract_base_classes/operators.py

from abc import ABCMeta, abstractmethod
from core.abstract_base_classes.attributes import AttributeBag
from collections.abc import MutableSet
from uuid import UUID


class Operator(metaclass=ABCMeta):
    """
    An abstract class defining the interface for an Operator entity
    representing a call center agent.
    """

    @property
    @abstractmethod
    def id(self) -> UUID:
        pass

    @property
    @abstractmethod
    def status(self):
        pass

    @status.setter
    @abstractmethod
    def status(self, value):
        pass

    @property
    @abstractmethod
    def attributes(self) -> AttributeBag:
        pass


class Pool(MutableSet):
    """
    An abstract class defining the interface for an operator pool.
    """

    @abstractmethod
    def __contains__(self, operator: Operator):
        pass

    @abstractmethod
    def discard(self, operator: Operator):
        pass

    @abstractmethod
    def __iter__(self):
        pass

    @abstractmethod
    def add(self, operator: Operator):
        pass

    @abstractmethod
    def __len__(self):
        pass

# core/Entities/Operator.py

from core.abstract_base_classes.operators import Operator as AbstractOperator
from core.abstract_base_classes.attributes import AttributeBag as AbstractAttributeBag
from core.abstract_base_classes.attributes import AttributeTag as AbstractAttributeTag
from core.Entities.AttributeBag import AttributeBag
from core import status
from uuid import UUID, uuid4


class Operator(AbstractOperator):
    def __init__(self, attributes: AbstractAttributeBag=None):
        if not attributes:
            attributes = AttributeBag()

        super().__init__()
        self._id = uuid4()
        self._attributes = attributes
        self._status = status.INITIALIZED

    @property
    def id(self) -> UUID:
        return self._id

    @property
    def status(self):
        return self._status

    @status.setter
    def status(self, value):
        self._status = value

    @property
    def attributes(self) -> AbstractAttributeBag:
        return self._attributes

    @attributes.setter
    def attributes(self, attributes: AbstractAttributeBag):
        self._attributes = attributes

    def add_attribute(self, attribute: AbstractAttributeTag):
        self._attributes.add(attribute)

# core/Entities/OperatorPool.py

from core.abstract_base_classes import operators


class Pool(operators.Pool):
    def __init__(self):
        super().__init__()
        self._operators = set()

    def __iter__(self):
        return self._operators.__iter__()

    def __contains__(self, operator: operators.Operator):
        return operator in self._operators

    def discard(self, operator: operators.Operator):
        self._operators.remove(operator)

    def add(self, operator: operators.Operator):
        self._operators.add(operator)

    def __len__(self):
        return len(self._operators)
\$\endgroup\$
4
\$\begingroup\$

I'm a bit worried about file structures and putting every concrete class in its own file, as that results in from Foo.Bar.Baz import Baz; is that acceptable, or is the common wisdom to include more classes in a single module?

There's absolutely nothing wrong with putting multiple classes in the same file, especially if it makes the code more readable. That said, there's nothing saying you can't re-export your classes from another file if you would like the code and the module structure to be somewhat orthogonalized.

I'm also using a lot of type hinting; while I'm aware that it doesn't actually do anything in Python, it does help me keep the ducks straight in my head while typing and helps my IDE help me. Any concerns here?

No concerns; this is a good thing to do from a documentation perspective and it's even better with IDE support.

I would be hesitant with core.abstract_base_classes; the standard name is the shortened ABC, so ABCs it would be. Even so, it seems strange to put all the ABCs in one place; why not put them with the classes that implement them?

In Operator, you do

if not attributes:
    attributes = AttributeBag()

This should be the explicit

if attributes is not None:
    attributes = AttributeBag()

since you're checking against a sentinel.

You also call super().__init__(); this seems pretty superfluous. You don't need to call super() and I'd avoid it unless it serves a purpose.

I can kind'a get why you'd want a property for id - to keep it read only. However, there's no reason to have status or attributes as a property. Don't, it doesn't buy you anything (and does have costs).

But this puts into question why you have the ABCs at all... why do you? If you had multiple classes, sure. You might want something to guide it. But as it stands, you're using it once and it's just overhead. Don't, it doesn't buy you anything significant (and does have costs).

Similar applies to Pool... but why not just use set? Not only does set do everything that Pool can do, it's better tested and supports more functionality. Further, everyone knows how to use set (I hope).

Don't build abstractions for the sake of abstractions. Build abstractions in order to simplify common functionality or allow extensibility where you plan to extend things. Abstractions are meant to make code simpler to understand, but if you build too many layers you drown out the intent that you're trying to express.

\$\endgroup\$
  • \$\begingroup\$ That's the thing, I'm in an exploratory phase and am not entirely sure yet whether I will need multiple implementations of an ABC or not, so I'd like to keep it clustered around interfaces. I guess it's not really as necessary in Python as in other languages though? You'd just punch the duck a bit more if you figure out you need more classes? What's the typical approach here? \$\endgroup\$ – deceze Jan 29 '15 at 12:09
  • \$\begingroup\$ Regarding the Pool: at the moment it's just a set, really, but I'm almost sure it'll get a few more methods in the future. Again though, maybe Python is dynamic enough to make later refactoring not really a big deal, and I should start with the simplest thing that could possibly work? \$\endgroup\$ – deceze Jan 29 '15 at 12:10
  • 1
    \$\begingroup\$ @deceze You don't need to write ABCs in advance since Python is dynamically typed. You can add an ABC later once you have multiple implementors if you wish (it's not even needed, just a nice-to-have). // Regarding Pool, if you actually plan on needing more state, do make a new class, but if you're just adding new "methods" they could just be free-standing functions. \$\endgroup\$ – Veedrac Jan 29 '15 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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