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.35k stars 148 forks source link

feat: skipDoubleRegistration configuration #356

Closed venkata-reddy-dev closed 7 months ago

venkata-reddy-dev commented 7 months ago

This PR is the implementation for feature request #335

Use case :

Some times developer don't want to touch production code to write test cases.

Scenario :

Code was release to production but some test cases are not written due to some high priorities. if developer want to write test cases after some time, Then some test mock implementations need to put in place of real implementations.

To do this developer need to add if-else cases in production code to place right dependencies for test environments. I feel touching production code after release, just for writing extra test cases is not a good approach.

 if (testing) {
    getIt.registerSingleton<AppModel>(AppModelMock()); /// this mocks are in production code  
  } else {
    getIt.registerSingleton<AppModel>(AppModelImplementation());
  }

Currently get_it does not have a capability to skip registrations if already registered. it throws Argument Error

Here is the test case which throws Argument Error


test(' Throws ArgumentError ', () async {
    final getIt = GetIt.instance;
    getIt.allowReassignment = false;
    getIt.reset();
    getIt.registerSingleton<DataStore>(MockDataStore());

    expect(
      () => getIt.registerSingleton<DataStore>(RemoteDataStore()),
      throwsArgumentError,
    );
  }); 

This PR adds one boolean variable skipDoubleRegistration to get_it configuration to ignore registration silently.

Here is the test case:

test(' Ignores ReassignmentError ', () async {
    final getIt = GetIt.instance;
    getIt.reset();
    getIt.allowReassignment = false;
    getIt.skipDoubleRegistration = true;
    getIt.registerSingleton<DataStore>(MockDataStore());
    getIt.registerSingleton<DataStore>(RemoteDataStore()); ///  Just ignores registration silently 

    expect(getIt<DataStore>(), isA<MockDataStore>());
  });

To understand more, i have implemented a real world use case in a sample git repo, look into source code

I am open to take feedback and happy to modify this PR to land this feature requirement into get_it package.

escamoteur commented 7 months ago

But why not simply setting allowReasignment== true? why a new property? what do I miss here?

escamoteur commented 7 months ago

OK, I now understood the goal. We should name it skipDoubleRegistration and mark it visibleForTesting

escamoteur commented 7 months ago

Also please also add it to the readme

venkata-reddy-dev commented 7 months ago

Okay I will do changes as requested

Thanks for the response

venkata-reddy-dev commented 7 months ago

@escamoteur changes are done as you suggested.

venkata-reddy-dev commented 7 months ago

@escamoteur can you review this changes and merge, if there is no code review comments.