bobthemighty / punq

An IoC container for Python 3.8+
MIT License
308 stars 14 forks source link

Callable singletons are registered incorrectly #6

Closed sobolevn closed 5 years ago

sobolevn commented 5 years ago

Bug report

Problem

Let's say I have a class like this:

class RunSomeQuery(object):
    def __init__(self, db):
         self._db = db
    def __call__(self, arg):
         return self._db.query(arg)

And then I register it as a singleton:

import punq

container = punq.Container()
container.register(RunSomeQuery, RunSomeQuery(db=my_db_instance))

The problem is that this line: https://github.com/bobthemighty/punq/blob/master/punq/__init__.py#L122 decides that this class is callable. And registers it as an implementation.

I still expect it to be registered as a singleton instance.

Proposed solution

Add a special kwarg to register singletons: container.register(BaseClassName, singleton=Instance()) Possibly, there are other solutions as well.

bobthemighty commented 5 years ago

Would you have this replace the existing support for singletons, or work alongside it?

Right now, I can do something like this:

class BitBucket:
    def consume_bits(self, bits):
        pass

bucket = BitBucket()
container.register(BitBucket, bucket)

and because the _factory type is not callable, we treat it as an instance. I guess we could replace that altogether, so that the only way to register a singleton instance is through the instance kwarg, ie.

container.register(BitBucket, instance=bucket)

That said, this is an edge case because the service itself is callable. Part of me likes the idea of testing for that.

BitConsumer = Callable[bytes]

def consume_and_log(data: bytes):
    print(bytes)

def consume(data: bytes):
    pass

def choose_consumer():
    return consume

# it would be nice if this just worked, for a whole bunch of scenarios.
container.register(BitConsumer, consume_and_log)

# but this makes it harder
container.register(BitConsumer, choose_consumer)
sobolevn commented 5 years ago

I think that singletons theyself are special cases. And should be treated differently.

This way one can decide how to use this class:

class RunSomeQuery(object):
    def __init__(self, db):
         self._db = db
    def __call__(self, arg):
         return self._db.query(arg)

As a factory or as a singleton. Which is not clear from the definition. And we can make a wrong decision.

bobthemighty commented 5 years ago

Hi @sobolevn - I've just opened #8 which provides an explicit kwarg instance for registering an explicit instance.

I've avoided the term singleton because it's not technically true. One explicit design goal for punq is to avoid global state, so you can create multiple containers each containing a different explicit instance.

container1 = Container()
container2 = Container()

container1.register(EmailSender, instance=SmtpEmailSender())
container2.register(EmailSender, instance=SmtpEmailSender())