fluttercommunity / get_it

Get It - Simple direct Service Locator that allows to decouple the interface from a concrete implementation and to access the concrete implementation from everywhere in your App. Maintainer: @escamoteur
https://pub.dev/packages/get_it
MIT License
1.36k stars 147 forks source link

[Question] GetItAll from one type? #75

Open rafaelcorbellini-egsys opened 4 years ago

rafaelcorbellini-egsys commented 4 years ago

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.

escamoteur commented 4 years ago

You mean you registered multiple instances of the same type? and you want to call the same method on all of them?

rafaelcorbellini-egsys commented 4 years ago

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 ?

escamoteur commented 4 years ago

ah, understood. I will think about it.

mareklat commented 3 years ago

@escamoteur Have you analyzed this topic? Are there any real chances to implement this?

escamoteur commented 3 years ago

Could you elaborated why this would be useful?

mareklat commented 3 years ago

Of course. Most of all, it helps to write modular and clean code.

For example: I have an Interceptor interface in my http client. In the object responsible for communication with the server, I inject a list of objects implementing the Interceptor interface. Then, when I want to add a new Interceptor, just only create a new implementation.

A similar solution is present in other DI libraries, e.g. Dagger, Spring Boot, etc.

escamoteur commented 3 years ago

One question would be should it only get all of exact the same type or also derived types?

mareklat commented 3 years ago

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

getit.factory<Interceptor>(() => LoggingInterceptor());

Currently, when I register more than one object with the same type and when allowReassignment==false , the library throws an exception. Instead, we could register a collection of interfaces

getit.factory<List<Interceptor>>(() => [
   get<LoggingInterceptor>(),
   get<AnotherInterceptor>(),
   ...
]);

Maybe new flag will be necessary to allow binding into collection.

escamoteur commented 3 years ago

ok, makes sense, but I need to find time for it. Still not finished to convert all my pacakges to null safety

mareklat commented 3 years ago

Fantastic :) I haven't explored the get_it internal yet, but in my free time I'll try to help

mareklat commented 3 years ago

@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

escamoteur commented 3 years ago

OK,

your PR used a special intoSet parameter. But do we need this if we can use names instances? You want to be ablt to do a .getAll<Type>() right? If we limit it to the exact type I would probably only have to make a smal change to the internal get_it data structure. But my feeling is we should be able to request all of one type plus derived types to really allo full flexibility

mareklat commented 3 years ago

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: getIt.registerFactory(() =>UserDTO())

But if we want to also register instance to collection of supertypes we can call like this:

getIt.registerFactory(() =>UserDTO()).inToList<DTO>();

and in some point in code:

getIt.get<List<DTO>>()

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:

class Multibinding<T extends Object> {

  final GetIt _getIt;

  Multibinding(this._getIt);

  void inToList<S>() {
    if (!(T is S)) {
      throw "$T is not subtype of $S";
    }

    if (_getIt.isRegistered<List<S>>()) {
      List<S> list = _getIt.get();
      _getIt.unregister<List<S>>();
      _getIt.registerFactory<List<S>>(() {
        T instance = _getIt.get();
        var newList = list.toList();
        newList.add(instance as S);
        return List.unmodifiable(newList);
      });
    } else {
      _getIt.registerFactory<List<S>>(() {
        T instance = _getIt.get();
        var newList = <S>[];
        newList.add(instance as S);
        return List.unmodifiable(newList);
      });
    }
  }

}

I think this solution allows you to implement multibinding concept with a minimum amount of work. What do you think?

escamoteur commented 3 years ago

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.

mareklat commented 3 years ago

Hmm.. this will be much better to use. In this approach when we call getAll<SuperType>(), we need scan all factories and compare like T is SuperType, collect them to list and return. This will be efficient?

escamoteur commented 3 years ago

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. it's O(n) which means even if you have registered 1000 objects it shouldn't take long

escamoteur commented 3 years ago

I would say lets implement it like that and see what the feedback is

escamoteur commented 3 years ago

would you try a PR?

mareklat commented 3 years ago

You're right, maybe it's not such a bad idea. Ok, I will prepare a PR and we will see :)

escamoteur commented 3 years ago

please don't forget adding the new function to the readme

ernesto commented 3 years ago

@mareklat I am working with a similar stuff here, your PR will be very appreciated :D About O(n) problem, I suppose it can be resolved with a getAll<SuperType, DerivadType?>() ?

vladimirfx commented 3 years ago

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 getAll<ExactType>() and see on feedback to implement withSubtypes case.

About O(n) complexity - for most practical graphs (<1000 bindings) is not measurable impact on app performance.

escamoteur commented 2 years ago

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

iandis commented 2 years ago

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.

0ttik commented 1 year ago

Also let's say I've registered some factories in get it and they all implement IFactory interface. Now I want to write some kind of factory selector and it would be perfect for me to get all factories and then select the appropriate one.

brunogabriel commented 1 year ago

Hey guys, sorry for revive this topic, but is there any plan to implement this feature?

praxamarnix commented 1 year ago

What I really enjoy using Autofac, is how they handle multiple with the As<> binding. You register an instance not only as itself, but also as a specific interface.

Requesting for this interface could return multiple instances or only one.

GetIt.I.register<Foo>(Foo()).As<IFoo>();
GetIt.I.register<Bar>(Bar()).As<IFoo>();

var allFoos = GetIt.I.getAll<IFoo>();

I know that Autofac is a dependency injection framework, but for me something similar could work here as well. I found another package called needle, that seems to do something similar but it hasn't been developed for over 2 years.

The interesting part for me here is that they return some kind of RegistrationBuilder class that again has more functions to work with to complete the registration.

I have not looked into the GetIt code base, but I can imagine that this would require quite some refactor.

escamoteur commented 1 year ago

What I really enjoy using Autofac, is how they handle multiple with the As<> binding. You register an instance not only as itself, but also as a specific interface.

Requesting for this interface could return multiple instances or only one.

GetIt.I.register<Foo>(Foo()).As<IFoo>();
GetIt.I.register<Bar>(Bar()).As<IFoo>();

var allFoos = GetIt.I.getAll<IFoo>();

I know that Autofac is a dependency injection framework, but for me something similar could work here as well. I found another package called needle, that seems to do something similar but it hasn't been developed for over 2 years.

The interesting part for me here is that they return some kind of RegistrationBuilder class that again has more functions to work with to complete the registration.

I have not looked into the GetIt code base, but I can imagine that this would require quite some refactor.

you are aware, that you can do exactly that if you pass your interface type as the generic parameter?

GetIt.I.register<IFoo>(Bar());
hughesjs commented 9 months ago

Any movement on this? It's a pretty crucial feature if you want to have a module based architecture

escamoteur commented 9 months ago

Passing an additional interface type is an interesting idea. Would make it easy to implement it performant. They get_it is currently implemented, getting all implentations of one type wouldn't be fast as I would have to go through all registrations. Passing an optional interface type when registering would be a clear signal to save a reference to that type parallel to the normal registration.

Am 11. Feb. 2024, 18:52 +0100 schrieb James H @.***>:

Any movement on this? It's a pretty crucial feature if you want to support plugins... — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.***>

escamoteur commented 9 months ago

It would be helpful to get some application examples for this feature. So far I always could decide while registering which implementation I need. Am 11. Feb. 2024, 19:00 +0100 schrieb @.***:

Passing an additional interface type is an interesting idea. Would make it easy to implement it performant. They get_it is currently implemented, getting all implentations of one type wouldn't be fast as I would have to go through all registrations. Passing an optional interface type when registering would be a clear signal to save a reference to that type parallel to the normal registration.

Am 11. Feb. 2024, 18:52 +0100 schrieb James H @.***>:

Any movement on this? It's a pretty crucial feature if you want to support plugins... — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.***>

hughesjs commented 9 months ago

It would be helpful to get some application examples for this feature. So far I always could decide while registering which implementation I need. Am 11. Feb. 2024, 19:00 +0100 schrieb @.: Passing an additional interface type is an interesting idea. Would make it easy to implement it performant. They get_it is currently implemented, getting all implentations of one type wouldn't be fast as I would have to go through all registrations. Passing an optional interface type when registering would be a clear signal to save a reference to that type parallel to the normal registration. Am 11. Feb. 2024, 18:52 +0100 schrieb James H @.>: > Any movement on this? It's a pretty crucial feature if you want to support plugins... > — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you modified the open/close state.Message ID: @.***>

So my use case is that I want to be able to register an OverviewScreenTabDefinition for each of the tabs I want to show up in my navigation bar. I then want to inject that entire list into a PageDefinitionProvider which my MainLayout uses to populate its navigation bar and content. More specifically, I want each module to independently register its own definitions into the container and have no knowledge of any other module. (Because yes I could have some central DI configurator that just manually registers the list, but then it would have to know about every module and couple everything together)

Another case I use this for in C# quite a lot is with strategy patterns where I can register an arbitrary number of strategies, inject that collection into a factory and the factory returns the first instance that .CanResolve() whatever situation it is facing.

escamoteur commented 9 months ago

One aspect I'm not clear is if I register multiple instance types under the same interface type how would we access one specific instance? Because currently the type you pass as generic when registering is the one to access the instance. Would passing an additional Super interface type when registering be enough for your applications?

Or would it be that the moment you register a second instance with the same type that you can only use getAll and no longer can access a single instance by type? Am 12. Feb. 2024, 12:13 +0100 schrieb James H @.***>:

It would be helpful to get some application examples for this feature. So far I always could decide while registering which implementation I need. Am 11. Feb. 2024, 19:00 +0100 schrieb @.: … Passing an additional interface type is an interesting idea. Would make it easy to implement it performant. They get_it is currently implemented, getting all implentations of one type wouldn't be fast as I would have to go through all registrations. Passing an optional interface type when registering would be a clear signal to save a reference to that type parallel to the normal registration. Am 11. Feb. 2024, 18:52 +0100 schrieb James H @.>: > Any movement on this? It's a pretty crucial feature if you want to support plugins... > — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you modified the open/close state.Message ID: @.> So my use case is that I want to be able to register an OverviewScreenTabDefinition for each of the tabs I want to show up in my navigation bar. I then want to inject that entire list into a PageDefinitionProvider which my MainLayout uses to populate its navigation bar and content. Another case I use this for in C# quite a lot is with strategy patterns where I can register an arbitrary number of strategies, inject that collection into a factory and the factory returns the first instance that .CanResolve() whatever situation it is facing. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.>

hughesjs commented 9 months ago

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<T>(), it's just the behaviour of .Get<IEnumerable<T>() (the equivalent of iterable).

escamoteur commented 9 months ago

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: @.***>

hughesjs commented 9 months ago

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 overload 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.

escamoteur commented 9 months ago

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: @.***>

hughesjs commented 9 months ago

Failing that, I don't think there's any qualms in having it as a separate .GetAll()

escamoteur commented 9 months ago

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: @.***>

escamoteur commented 9 months ago

One important question wasn't discussed yet. Should getAll() 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: @.***>

hughesjs commented 9 months ago

I would expect it to only return things that were explicitly registered with that interface:

So:

.register<BaseType>(DerivedA());
.register<BaseType>(DerivedB());
.register<DerivedC>(DerivedC());
.getAll<BaseType>();

Would return:

[
DerivedA(),
DerivedB()
]
hughesjs commented 9 months ago

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

escamoteur commented 9 months ago

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 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: @.***>

hughesjs commented 9 months ago

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

escamoteur commented 9 months ago

Hi, I'm implemented that feature but haven't tested it yet. See https://github.com/fluttercommunity/get_it/pull/354 It would be awesome if one of you could write some tests for it and and a section in the readme.

escamoteur commented 9 months ago

Also if one of you could review my latest changes in case you spot any problems. Existing tests pass all.

escamoteur commented 9 months ago

The current implementation only returns the current Scope, I all an allScopes parameter as soon as you can verify that this works so far

hughesjs commented 9 months ago

I'll take a look if I get a chance, might not be til the weekend though

escamoteur commented 9 months ago

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: @.***>

tpucci commented 8 months ago

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 ?