-
-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Question] GetItAll from one type? #75
Comments
You mean you registered multiple instances of the same type? and you want to call the same method on all of them? |
I want get all instance of one type. Example i have N InterfaceDAO (UserDao, TodoDAO... ) and register all of them. have some way to resolve all (Iterable) registered ? |
ah, understood. I will think about it. |
@escamoteur Have you analyzed this topic? Are there any real chances to implement this? |
Could you elaborated why this would be useful? |
Of course. Most of all, it helps to write modular and clean code. For example: A similar solution is present in other DI libraries, e.g. Dagger, Spring Boot, etc. |
One question would be should it only get all of exact the same type or also derived types? |
I think they should it only get all of exact the same type. We can use type passed when there are registering factory, for example
Currently, when I register more than one object with the same type and when
Maybe new flag will be necessary to allow binding into collection. |
ok, makes sense, but I need to find time for it. Still not finished to convert all my pacakges to null safety |
Fantastic :) I haven't explored the get_it internal yet, but in my free time I'll try to help |
@escamoteur Hey, have you found the time to focus on this idea? Can we reopen this topic? I believe that this is a crucial and much needed feature |
OK, your PR used a special |
Yes, I agree that the ability to call derived types should remain. I have a few ideas, but most of it comes to idea where firstly we register instance in the same way as it is now, and optionally bind it to list of parent types. Normally we can register instance like this: But if we want to also register instance to collection of supertypes we can call like this:
and in some point in code:
To allow this behavior, each registration method must return an object. An object that allows you to call the inToSet method. An example of this object:
I think this solution allows you to implement multibinding concept with a minimum amount of work. What do you think? |
not sure if we need that. if we add an List<MyType> = getIt.getAll<MyType>(); that returns you all registered objects with that type and its derived types. |
Hmm.. this will be much better to use. |
Yeah this might not be very performant. But I try to imagine how often will you have to do such a call? It's probably nothing you do on every frame. |
I would say lets implement it like that and see what the feedback is |
would you try a PR? |
You're right, maybe it's not such a bad idea. Ok, I will prepare a PR and we will see :) |
please don't forget adding the new function to the readme |
@mareklat I am working with a similar stuff here, your PR will be very appreciated :D |
IMO there is two separate API calls:
Our practice with Dagger and Guice (Android and backend) shown that first call is sufficient in 90%+ cases. Actually I found 4 cases out of few hundred in all accessible projects where with subtypes lookup is used. For SOLID-complied code that case is very uncommon if possible at all. So I propose implement '10 for 90' case with just About O(n) complexity - for most practical graphs (<1000 bindings) is not measurable impact on app performance. |
Hi, sorry for not looking into this earlier, but I had some mental health problems the last half year. No PR has appeared yet. let me see if I find the time to add it |
I think a use case like clearing/resetting all the DAOs/ViewModel objects when a user log out can be done by using something like the Pub-Sub pattern instead. |
I don't think you need to overload anything, both of those method calls have the same signature, one type arg and nothing else, there's probably just an As for your second bit, do you mean if you want to register multiple implementations and get a specific one you need a name? I don't see why you'd need a name if you only ever wanted to be able to get the list. Another option would be to just document that if you register multiple and don't provide instance names, you get the first/last on registered when trying to get a single one. |
Will have to see if Dart allows me that especially extracting the type of tge enumerable and use it to lookup the registered type inside its Map.
Yeah, true, it wouldn't be absolutely necessary to provide a name.
Will think about it a bit more
Am 12. Feb. 2024, 14:43 +0100 schrieb James H ***@***.***>:
… > The later one probably because c# supports method overloading. Maybe making it a requirement to provide an instance name if you want to register multiple implementations and get all of them would make implementing this really easy and fast Am 12. Feb. 2024, 14:36 +0100 schrieb James H @.>:
> …
> I think the way this is handled in MS DI is that if you want a specific implementation when there are multiple registered you need to use a named instance, or register a factory which takes the list and does what you need with it. In all honesty, in cases where I've used this, I've never had a case where I've wanted to inject both a list of all implementations and a specific implementation. So I'm not 100% sure if what I'm saying is correct. Interestingly, there's also no concept of .GetAll(), it's just the behaviour of .Get<IEnumerable() (the equivalent of iterable). — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.>
I don't think you need to override anything, both of those method calls have the same signature, one type arg and nothing else, there's probably just an if typeof(T).IsAssignableTo<IEnumerable<>> in there.
As for your second bit, do you mean if you want to register multiple implementations and get a specific one you need a name? I don't see why you'd need a name if you only ever wanted to be able to get the list.
Another option would be to just document that if you register multiple and don't provide instance names, you get the first/last on registered when trying to get a single one.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Failing that, I don't think there's any qualms in having it as a separate |
Hey, would you be willing to add tests and a new chapter for the Readme when I implement this?
Am 12. Feb. 2024, 14:56 +0100 schrieb James H ***@***.***>:
… Failing that, I don't think there's any qualms in having it as a separate .GetAll()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
One important question wasn't discussed yet. Should getAll<Interface>() return only objects that were directly registered with that type or also return objects that were registered with derived types of Interface?
Am 12. Feb. 2024, 18:17 +0100 schrieb ***@***.***:
… Hey, would you be willing to add tests and a new chapter for the Readme when I implement this?
Am 12. Feb. 2024, 14:56 +0100 schrieb James H ***@***.***>:
> Failing that, I don't think there's any qualms in having it as a separate .GetAll()
> —
> Reply to this email directly, view it on GitHub, or unsubscribe.
> You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
I would expect it to only return things that were explicitly registered with that interface: So:
Would return:
|
An old colleague of mine has just shared this with me which I believe offers the functionality we're talking about. Might be helpful, might not. |
I just started implementing this but I'm not clear how we should tread Scopes in this context. In therie it is possible to register to instances of a type in two different scopes. Calling normal get<Type> takes the first one it finds before checking underlying sopes.
Should getAll() only work on the active scope or collect all instances of all scopes?
Nachricht von James H ***@***.***> am Feb 13, 2024, 1:40 PM +0100:
… An old colleague of mine has just shared this with me which I believe offers the functionality we're talking about. Might be helpful, might not.
https://github.com/LeeSanderson/SimpleDartServiceProvider
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
I'm not sure about that one. I think the principal of least surprise would suggest either of these would be acceptable:
I've not really used this feature though |
Hi, |
Also if one of you could review my latest changes in case you spot any problems. Existing tests pass all. |
The current implementation only returns the current Scope, I all an |
I'll take a look if I get a chance, might not be til the weekend though |
NP, I try to add the allScopes till then too
Am 20. Feb. 2024, 10:28 +0100 schrieb James H ***@***.***>:
… I'll take a look if I get a chance, might not be til the weekend though
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Hey! I've tried the feature. There is a little bug and the fix is easy. Otherwise, it seems good! 🙂 Should I make a PR ? |
Yes please. I have to add the option to return all instances of all scopes. I hope to do it tomorrow
Am 24. Feb. 2024, 18:00 +0100 schrieb Thomas Pucci ***@***.***>:
… > Hi, I'm implemented that feature but haven't tested it yet. See #354 It would be awesome if one of you could write some tests for it and and a section in the readme.
Hey!
I've tried the feature. There is a little bug and the fix is easy.
Otherwise, it seems good!
🙂
Should I make a PR ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Here you go: #355 |
Can we merge this @escamoteur ? 🙂 #355 |
Sorry, had no time yet, will do today,
Am 21. März 2024, 09:13 +0100 schrieb Thomas Pucci ***@***.***>:
… Can we merge this @escamoteur ? 🙂 #355
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
if anyone of you have time to add some docs on this new function (we now also have the async version) #361 to the readme, I would be really thankful as I don't have a lot of time at the moment |
Yeah, I can probably document this (or at least start on documenting it) this afternoon. |
That would be awesome. currently I didn't have time to add the al |
Is this all merged into main? Just so I know where to branch from. |
yep |
How's this? #363 |
Hi friends, may I ask you for one more help on this? I just added the |
Sorry, I've not really used the scopes functionality of GetIt, so I wouldn't be the best person to write this |
No problem
Nachricht von James H ***@***.***> am May 14, 2024, 2:27 PM +0200:
… Sorry, I've not really used the scopes functionality of GetIt, so I wouldn't be the best person to write this
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Is there any way to get everyone of a kind?
example when logging out of the application I want to get all objects registered as DAO and perform a function of them.
The text was updated successfully, but these errors were encountered: