Di docs #98
Di docs #98
Conversation
all we really want for Key is instance control. the previous implementation would cause horrible problems for pairs of (type, annotation)s where the hash codes collide, which is easy given the poor choice of hash function (a sum of the hash codes of the components). also, lastKeyId was a misnomer, since it actually held the number of instances. this wrong name caused off-by-1 usage, which didn't cause a bug, but was misleading and wasted a little memory. i believe this implementation to be vastly superior in memory usage since it returns cached Keys instead of creating new, equivalent ones. added and simplified Key tests. docs(Injector, etc): Provided accurate docs in many places. refactor(ResolutionContext): Small changes. - added ResolutionContext type annotation where it was missing. - changed const ResolutionContext to new, since const doesn't work in general (it only works when the key is null). - simplified docs to ResolutionContext and made them more useful (imo). refactor(base_injector.dart): Rename _ProviderWithDefiningInjector... to shorter _ProviderWithInjector since it's just a pair.
| @@ -25,7 +25,7 @@ class DynamicInjector extends BaseInjector { | |||
| new DynamicInjector._fromParent(modules, this, name: name); | |||
|
|
|||
| Object newInstanceOf(Type type, ObjectFactory objFactory, Injector requestor, | |||
| resolving) { | |||
| ResolutionContext resolving) { | |||
|
Most of the changes are great some are useless IMO. Could you please squash your commits and improve the commit comments ? (you can then force push to this same branch, no need to open a new PR). Ideally I would see 3 commits here:
|
|
You also might want to update the PR title |
|
@markovuksanovic wrong PR number, will update! |
| Map<int, int> _hashToKey = {}; | ||
|
|
||
| /** | ||
| * Key to which an [Injector] binds a [Provider]. This is a pair consisting of |
There was a problem hiding this comment.
You use Provider a lot in comments... Provider has always been an internal implementation detail, it's not exposed in any public API. I would rather use the term "binding".
There was a problem hiding this comment.
we discussed this offline, but posting here for posterity:
Provider is publicly exposed via NoProviderError and Module.bindings, which is a map from int to Provider. it's also tedious to try to explain most of this di design without this concept. e.g. an injector binds a key to a ... what? not an instance.
it is an admirable goal to see if one can hide this concept, but i think it would require a substantial redesign. if that happens, just alter the docs at that point.
No description provided.