You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I already made my points in #301, I'll just copy all of it here.
I should probably mention that we've already been using the DI system in production for 6+ months (ever since I contributed the first version of the code). We're using a custom build that injects entire records, since we use those as service interfaces, which helps a lot when mocking services for tests. We haven't run into any obstacles yet.
Injecting records looked like a good idea at first, since assigning names to parameters is a good thing. However, it causes a very big problem: What if we wanted to inject an entire record?
In F#, there are (AFAIK) two ways to create abstract sets of coherent operations:
Use interfaces, and implement them with concrete types. Store data in fields.
Use records containing functions. Implement operations by creating new instances of the record. Store data in closures.
Without going into detail about the pros and cons of each approach, I think we shouldn't force users of Saturn into using interfaces; and I can also think of a few other situations where injecting records could be useful.
Now, if we tried to inject this record:
typeMyService={
AddInts:int -> int -> int}letmyServiceFactory(logger:ILogger<MyService>)={
AddInts =fun x y ->
logger.LogInformation("Adding two ints")
x + y
}
We'd get an error at runtime saying something like this: No registered service for FSharpFunc<int, int, int>. This is, of course, due to the dependency resolution code attempting to resolve each field of a record separately.
On the other hand, the argument for resolving record fields boils down to being able to name each dependency individually. After all, who'd want to write this?
get_di "/"(fun(d: IMyService *IMyOtherService*ILogger*IHttpClientFactory)->letmyService,myOtherService,logger,httpClientFactory = d
...
It's very highly unlikely that two functions would have the exact same dependencies. Record field resolution is best used with anonymous records:
There's also the fact that most people probably don't even expect fields of their record types to be resolved individually.
Given all of this, I feel the dependency resolution logic shouldn't attempt to resolve record fields. This change removes that logic and fixes the tests accordingly. I also modified the tuple tests to use individually named arguments, which looks much better than deconstruction.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
I already made my points in #301, I'll just copy all of it here.
I should probably mention that we've already been using the DI system in production for 6+ months (ever since I contributed the first version of the code). We're using a custom build that injects entire records, since we use those as service interfaces, which helps a lot when mocking services for tests. We haven't run into any obstacles yet.
Injecting records looked like a good idea at first, since assigning names to parameters is a good thing. However, it causes a very big problem: What if we wanted to inject an entire record?
In F#, there are (AFAIK) two ways to create abstract sets of coherent operations:
Without going into detail about the pros and cons of each approach, I think we shouldn't force users of Saturn into using interfaces; and I can also think of a few other situations where injecting records could be useful.
Now, if we tried to inject this record:
We'd get an error at runtime saying something like this:
No registered service for FSharpFunc<int, int, int>
. This is, of course, due to the dependency resolution code attempting to resolve each field of a record separately.On the other hand, the argument for resolving record fields boils down to being able to name each dependency individually. After all, who'd want to write this?
It's very highly unlikely that two functions would have the exact same dependencies. Record field resolution is best used with anonymous records:
However, this can more easily be accomplished using tuples like this:
There's also the fact that most people probably don't even expect fields of their record types to be resolved individually.
Given all of this, I feel the dependency resolution logic shouldn't attempt to resolve record fields. This change removes that logic and fixes the tests accordingly. I also modified the tuple tests to use individually named arguments, which looks much better than deconstruction.
Beta Was this translation helpful? Give feedback.
All reactions