Should None be an allowed factory return value?

[elfjes]

currently, Container.get starts with the following

if (svc := self._instantiated.get(svc_type)) is not None:
    return svc

meaning that if a factory has been run, and returns None, this value is not considered cached when requesting it the next time and the factory will be run again, overriding any potential closing logic.

I could imagine a user wanting to do something along the lines of:

def int_factory():
    yield None
    ... # some cleanup logic


qol.register(int, int_factory)

# later
result = qol.get(int)
if result is not None:
    ... # some logic
else:
    ... # some other logic

I'm not sure exactly what a use case for this might be, but the current implementation disallows the user to have None be a meaningful value which is generally considered not a good practice for a library. And it's easy to fix by just substituting the first dictionary lookup default value by a sentinel

[hynek]

Noe that's not what it means. self._instantiated is the container's cache and get() is not None means that something is already cached – or am I missing something?

[elfjes]

Let's say a user has set up a factory that explicitely returns None for a particular Service (under certain conditions), and wants to cache that value in the Container. Currently, this value is properly being cached in self._instantiated. However, the second time the Service is requested, that first if-statement resolves to False and the cached value is not used. Instead, the instantiation logic is run again, which may result in undesirable behaviour.

Now, what I said before about closing logic was erroneous, since all generators are added to the _on_close list so that all cleanup logic is run. but the above reasoning still stands I think

[hynek]

But what would None mean? Should we raise an exception if a factory returns None?

Aren't you overarchingly asking for #12?

[elfjes]

That's the thing. I don't think it should be up to the library (ie. svcs) to determine what None means, that's for the application developer to decide. Would you see value in if svcs just transparently accepts a None coming from a factory and subsequently caches and reuses it as if it were any other value? Right now, it doesn't reuse a cached None

[hynek]

ah now I got it 😅

currently we're being kinda smart – yeah we should fix that

[hynek]

fix is in 23.11!

[elfjes]

Great ^^