ncraike / fang

Dependency injection system for Python
MIT License
15 stars 1 forks source link

Creating a DependencyResolver with default arguments raises an error #12

Open ncraike opened 7 years ago

ncraike commented 7 years ago

The DependencyResolver's __init__ takes two arguments, dependency_register and resource_provider_register, which both have default values of None:

class DependencyResolver:

    def __init__(
            self,
            dependency_register=None,
            resource_provider_register=None):
        self.dependency_register = dependency_register
        self.resource_provider_register = resource_provider_register

        # Methods delegated to other objects
        self.query_dependents_resources = \
                self.dependency_register.query_resources
        self.resolve = self.resource_provider_register.resolve

However, actually using these default values causes errors to occur:

        self.query_dependents_resources = \
>               self.dependency_register.query_resources
E       AttributeError: 'NoneType' object has no attribute 'query_resources'

This is due to our method "delegation" failing completely on a None value.

No code within fang itself seems to trigger this behaviour. We only construct DependencyResolver ourselves in the Di class, and we provide values for these arguments:

class Di:

    def __init__(self, namespace=None):
        self.namespace = namespace
        self.dependencies = DependencyRegister()
        self.providers = ResourceProviderRegister()
        self.resolver = DependencyResolver(
                dependency_register=self.dependencies,
                resource_provider_register=self.providers)
ncraike commented 7 years ago

The usage seen in Di is the intended, typical, and possibly only valid usage of DependencyResolver: the code constructing the DependencyResolver must provide instances of DependencyRegister and ResourceProviderRegister to be used. Without these instances DependencyResolver cannot perform its task: it has no registers to query dependencies or resolve providers.

I'm unsure what behaviour would be appropriate if both of these instances aren't given. Potentially the DependencyResolver could construct its own DependencyRegister and ResourceProviderRegister instances if none are given, but I think this would be an unnecessary complication. Constructing appropriate register instances and combining them with the resolver is already the responsibility of the Di class. This would also lead to further complexity when namespace support is expanded.

Finally, it would actual be unusual to construct the DependencyResolver directly without using the Di class.

As such, the best solution to this bug may be to remove the default values for both __init__ arguments. The __init__ method should take ready DependencyRegister and ResourceProviderRegister instances as positional arguments.