injector icon indicating copy to clipboard operation
injector copied to clipboard

Using default arguments when nothing is injected

Open davidparsson opened this issue 5 years ago • 7 comments

Using NewType (or Key) is a great way to inject configuration into classes, but there are cases where there is a sane default that is rarely overridden. In these cases it would be nice to have a way to define the default value close to where it is used.

Injector currently does not consider default values unless @noninjectable is used, but @noninjectable does not allow configuration to be injected when the default is not to be used.

Is there a way to do this? If not, what are your thoughts on the topic?

The following is an example of how it would be useful:

SleepTime = NewType('SleepTime', float)


class WaitStrategy:

    @inject
    def __init__(self, sleep_time: SleepTime = 1.0) -> None:
        self._sleep_time = sleep_time

davidparsson avatar Jun 14 '19 12:06 davidparsson

Making Injector consider default arguments might get unwanted consequences for optional arguments (where None is the default), so it's possible that a new decorator would be a better solution for this.

davidparsson avatar Jun 14 '19 12:06 davidparsson

That being said, using the default values would be intuitive in most cases.

davidparsson avatar Jun 14 '19 12:06 davidparsson

+1 on always respecting the function-definition default for any type which has not had an injectable configure for it explicitly. The current behavior is, in effect, silently overriding the default of every optional argument. If I have a function with the argument my_var: Optional[str] = None and I call it without supplying a value for my_var, I expect my_var to be None, not the empty string!

macdjord avatar Feb 21 '20 20:02 macdjord

#116 is also related to that.

davidparsson avatar Feb 21 '20 20:02 davidparsson

I'd like to understand the use case for the parameter to both be injectable and have a default value. As I can't think of any I'd welcome any information here.

Raising an exception when injectable parameter (no matter if its type was bound explicitly or not) with default value is encountered seems like an option to me to avoid confusion and make things clear(er).

jstasiak avatar Feb 21 '20 20:02 jstasiak

@jstasiak, the short answer from me is primarily for configuration.

My use case was that I had some default string/numeric configuration that normally shouldn't be overridden, except with specific values for integration tests.

davidparsson avatar Feb 21 '20 21:02 davidparsson

While raising for a default value would remove the confusion, it will remove the ability to have default values which might complicate setup in tests when injector is not used. I believe that fixing the issue described in #116 would remove most of the risk of confusion here as well, since default values are primarily used for "value types".

davidparsson avatar Feb 21 '20 21:02 davidparsson