Register factories that take parameters

[mvarnagiris]

I would like to register factory that takes a parameter and use GetIt to get instance with parameter.

class SomeSource {
  final String someId;
  SomeSource(this.someId);
...
}

getIt.registerFactory((String someId) => SomeSource (someId));

getIt.get<SomeSource>("someId");

Hope it makes sense. If you want more info I can provide.

[escamoteur]

to make this types ave it would have to be a separate registration function which wouldn't be a big deal, but I'm not sure if it's a good idea to establish another get method with a parameter.

Maybe adding a new method 'newInstanceOf<FactoryType,paramType>(paramValue)' for registered factories could be a solution. What do you think?

@anaisbetts what do you think?

[mvarnagiris]

On Android at the moment I'm using Shank SL. It allows up to 4 parameters to be passed which is usually enough. Even supporting 1 parameter could be enough as you can always wrap your parameters. This should work with singletons as well. So if you have

getIt.registerLazySingleton((String) => MySingleton());

1) getIt.get<MySingleton>("1");
2) getIt.get<MySingleton>("2");
3) getIt.get<MySingleton>("1");

it should return 2 different instances. 1 and 3 would be same instance and 2 would be different instance

[escamoteur]

IMHO this goes a bit against the idea of a singleton :-)
If you need more than one Singleton with different properties you should just use named registration.

My problem is with that that then you always will have to use a (_) => in theregistrations if there is no parameter expected. And also always supply a second generic type

It would have to look like this:

getIt.registerLazySingleton<MySingleton,String>((s) => MySingleton(s));

How do we ensure type safety in your above get examples?

[MisterJimson]

What about for non-singleton factories? Like a ViewModel. They would often need different params passed in for new instances each time.

[anaisbetts]

The thing is, the whole idea of registration is that you register an interface, and the thing created is a concrete type. Interfaces inherently have no concept of constructors (though you could argue that abstract base classes do). If you have 5 different implementations of FooInterface, you can't say in the definition, "They all have to be constructable with these 3 parameters"

If you are creating a parameterized ViewModel, you should just use new because like, in a test runner or other different environment you're not replacing this with a stand-in, it's the thing you want to actually test!

Maybe I'm just not thinking through all of the use-cases, but in general it seems like if you have a need to use this feature, you might be putting the Wrong Things into service location, instead of "Things that vary based on Dev/Prod/Test".

[MisterJimson]

@anaisbetts I used to work with Xamarin and used AutoFac. I would register a bunch of objects in my container. Some wouldn't require any parameters, some would require other objects in the container as a dependency in the constructor, and others (ViewModels) would require other objects and also have some way to pass an argument (Usually with an init method that would be called right after it's created).

The reason I would normally throw everything in my container/locator is that I can choose which of the dependent objects I want Real and which I want Mock. Most often everything but the ViewModel would be mocked to test the ViewModel itself. However in a larger, end to end or integration test, I would sometimes want more of my objects to be real and less mocks. (Real database, Mock API, as an example).

So I guess I am just used to putting everything in a DI container and having it pull in what it needs to create an object and I am trying to mimic that here. Perhaps I should be thinking of things differently instead of trying to use just a ServiceLocator where I want DI? Not sure.

[anaisbetts]

Ah! So, the best pattern to do this is via optional parameters:

MyCoolClass({SomeDependency dep1, AnotherDependency dep2}) {
  this.dep1 = dep1 ?? getIt.get<SomeDependency>();
  this.dep2 = dep2 ?? getIt.get<AnotherDependency>();
}

In production this is a parameterless constructor, and in a test you just use new and pass in dep1 or dep2 as needed

[votruk]

As a user of this library, I would very appreciate an ability to add parameters for registering factories. In a manner of what @escamoteur proposed.

This possibility would make my code much cleaner. I use BLoCs, one per page. Sometimes I need to pass arguments there like an id of some object I want to display or smth else. Without this ability, I am forced to spoil my code with the details I don't want to see. Eg creation of some of the blocs can look like that:

_bloc = MyBloc(
    "some_recording_id", 
    getIt.get<UserRepository>(), 
    getIt.get<RecordingsRepository>(), 
    getIt.get<ConnectedDevicesRepository>(), 
    getIt.get<UploadRecordingsRepository>(), 
    getIt.get<UploadVideosRepository>(),
);

Instead of that what I really want is to divide this into two parts. First one should replace the code from above because it's the only one thing that I should care about during BLoC creation:

_bloc = getIt.get<MyBloc>(firstParam: "some_recording_id");

And the second part should be declared where all my classes set up:

_bloc = getIt.registerFactory<MyBloc, String>((String recordingId) => 
    MyBloc(
        recordingId, 
        getIt.get<UserRepository>(), 
        getIt.get<RecordingsRepository>(), 
        getIt.get<ConnectedDevicesRepository>(), 
        getIt.get<UploadRecordingsRepository>(), 
        getIt.get<UploadVideosRepository>(),
    ));

This helps me to keep my pages and widgets clear from all the dependencies that needed for BLoC only. And it easier to work with such code. Creation of all your business logic classes stored in one place.

I understand that maybe this approach does not match with the pure concept of Service Locator pattern., but I don't think that a good library should have this aim in mind. Especially when there are not a lot of alternatives you can find.

[escamoteur]

@votruk Would it be ok for you if this would be a separate registration function, so that the existing one wouldn't be changed? Otherwise we would force user always to pass a factoryFunction with a dummyID?

[votruk]

@escamoteur I think it would be a wise decision taking into account backwards compatibility.

The main question is about what methods should look like. Should it be like that for getters:

getIt.getWithArg<T>(dynamic arg, [String instanceName]);
getIt.getWithArg2<T>(dynamic arg1, dynamic arg2, [String instanceName]);
getIt.getWithArg3<T>(dynamic arg1, dynamic arg2, dynamic arg3, [String instanceName]);
...
getIt.getWithArgs<T>(List<dynamic> args, [String instanceName])

Or it would be better to make types explicit?

getIt.getWithArg<T, TArg>(TArg arg, [String instanceName]);
getIt.getWithArg2<T, TArg1, TArg2>(TArg1 arg1, TArg2 arg2, [String instanceName]);
getIt.getWithArg3<T, TArg1, TArg2, TArg3>(TArg1 arg1, TArg2 arg2, TArg3 arg3, [String instanceName]);
...
getIt.getWithArgs<T>(List<dynamic> args, [String instanceName])

What about instance names? Don't you think that it would be clearer to use named optional parameters instead?

getIt.getWithArg<T, TArg>( TArg arg, {String instanceName});
getIt.getWithArg2<T, TArg1, TArg2>(TArg1 arg1, TArg2 arg2, {String instanceName});
getIt.getWithArg3<T, TArg1, TArg2, TArg3>(TArg1 arg1, TArg2 arg2, TArg3 arg3, {String instanceName});
...
getIt.getWithArgs<T>(List<dynamic> args, {String instanceName})

Registration:

getIt.registerFactoryWithArg<T, TArg>(T Function(TArg arg) func, {String instanceName});
getIt.registerFactoryWithArg<T, TArg1, TArg2>(T Function(TArg1 arg1, TArg2 arg2) func, {String instanceName});
getIt.registerFactoryWithArg<T, TArg1, TArg2, TArg3>(T Function(TArg1 arg1, TArg2 arg2, TArg3 arg3) func, {String instanceName});
...
getIt.registerFactoryWithArg<T>(List<dynamic> args, {String instanceName})

Another question. Should we allow to register only factories or lazySingletons too?

[mvarnagiris]

I'd make types explicit and would allow to register lazySingletons. What would be a reason not to? Question is how many parameters will you support. On Android I use a SL library that allows up to 3 parameters and it's usually enough. In rare cases when it's not you can always create wrapper model that cas all the required parameters in there

[votruk]

@mvarnagiris
With Singletons, it can be trickier because if you'll have two calls in different places, you can get not expected result. See into this example:

class NameProvider {
    final String _name;

    NameProvider(this._name);
    
    String get getName => _name;
}

getIt.registerLazySingletonWithArg<NameProvider,  String>((name) => NameProvider(name));

class First {
    String getNameForFirst() => getIt.getWithArg<NameProvider, String>("First class").getName();
}

class Second {
    String getNameForSecond() => getIt.getWithArg<NameProvider, String>("Second class").getName();
}

void notObviousBehaviour() {
    print(First().getNameForFirst());
    print(Second().getNameForSecond());
    // you'll get the output:
    // "First class"
    // "First class"
}

In this example, if the SecondClass would be instantiated before the FirstClass, the output would be completely different.

But imagine this behaviour in a big app. Sometimes it would be very hard to understand what value would be in what moment there.

I sympathise for the idea of creating lazy singletons with arguments. But I don't think that we should incorporate functionality that can lead to potential bugs.

Maybe you have any solution to the problem and I'm just overreacting? Please share your thoughts.

[mvarnagiris]

But surely those lazy singletons should be singletons for those specific parameters. So it should work correctly. This would allow you to introduce "scoped" factories. Where you register a singleton within a specific scope. When that scope is destroyed - all instances in that scope are destroyed.

[escamoteur]

I don't see really the sense in allowing it for the lazy singleton as its a singleton, so not multiple instances.
I lean to make one parameter dynamic so that get()acn be used as always and checking the type at runtime inside get. If you need more than one parameter you can always pass a list or a wrapper object.

[escamoteur]

So I would go for

getIt.get<T>( {String instanceName, dynamic arg});

[mvarnagiris]

So for example imagine I have interface for NetworkClient and in my app I need to have 2 configurations of that NetworkClient. Say one is MY_API and another is THIRD_PARTY_API. Both of those network clients needs to be singletons. And I need 2 of them. How would I achieve that if I can only ask for NetworkClient with a parameter?

[escamoteur]

If you need two of the same time you can use named registration.

[mvarnagiris]

But with the way I am suggesting you can remove the need for "named". In essence exchange String for an actual meaningful type. Would that not be better? Seems like with named you already have a way of registering multiple instances of a singleton. Why is that supported but you also say:

I don't see really the sense in allowing it for the lazy singleton as its a singleton, so not multiple instances.

I'm probably missing something here

[escamoteur]

Identifiying a singleton by a combination of its own and its parameters does not feel right for me.

[mvarnagiris]

OK then there's no point in arguing really. I'm happy if this issue is closed.

[escamoteur]

Oh I overread your comment that you want to add a scope to a singleton registration.
That's an interesting thought, but I think it would be a different issue or not?

[escamoteur]

I don't say that I'm against parameters. Actually especially for flutter this would enable to pass a BuildContext to a factory. I'm just not sure how it would fit to the singletons, but maybe I just miss something here.

[mvarnagiris]

Ideally a scope to any instance. The way I would like to register instances is this:

getIt.registerSingle...
getIt.registerScoped...
getIt.registerNew...

Technically everything can be lazy (but maybe a parameter eagerInitialization can be supported). You can only get scoped instances by passing a Scope or maybe if you are within a Scoped class. I'm pretty new to Dart, so I'm not entirely sure if that is possible. I'm mostly working with Kotlin and it's how I use Service Locator there.

Regarding parameters - yes I think you should add support for them at least somewhere. I do believe that singletons should also support that, but it's a separate question - that's why I leave closing this issue to you - as this issue seems like 3 issues:

1. Parameter support
2. Scope support
3. Parametric singleton support (or whatever the term for that would be :)

[escamoteur]

Scoping will have to be good through through. Parameter support I will include in the next release

[escamoteur]

Included in #46