Annotation on data repository could be removed

[TonyTangAndroid]

In current implementation of Data Repository, let's say NoteDataRepository.java, the constructor has annotation @Inject and the class name has annotation Singleton.

However, if I remove this two annotations, the app will still compile and work as expected. And if I use Named annotation in DataModule.java, multiple NoteDataRepository instances will be created. So the @Singleton annotation in NoteDataRepository.java is not working as it was indicated.

Maybe my understanding for the purpose of annotations in NoteDataRepository.java is wrong. If not, I reckon that I should remove all the annotation in data repository since it is misleading. It has confused me for a while when I was trying to learn how to use Dagger.

PS: You could refer to this branch to prove that

multiple NoteDataRepository instances will be created. The code in NoteDataRepository.java constructor will be executed multiple times.

Log.e("restApi", "new NoteDataRepository");

[jordifierro]

Hi @TonyTangAndroid! I've been playing with the problem you comment.

The application is working as desired, creating and providing only one instance of NoteDataRepository for all UseCases that need it, as long as you keep the @Singleton annotation on the DataModule specific provider. Although it's true that @Singleton annotation is not needed on the NoteDataRepository class itself (it will compile and work as expected without it), I think it's necessary to keep it in order to document its instantiation policy. If you prefer to make the annotation mandatory, I suggest you to create a custom scope, since that kind of annotations will break the compilation when they differ from the provider.

In the case of @Inject, it is a bit similar. The NoteDataRepository annotation is just there to document (it works without it) because it's a chained injection. However, a root @Inject (for example @Inject NotesPresenter on NotesFragment) it's absolutely necessary to make Dagger to solve this and its child dependencies. As in the scope case, I recommend to keep @Inject to an easier understand how each class will be instantiated.

Hope it helps! Let me know if you don't agree or understand something of what I have exposed here.

[TonyTangAndroid]

Hi @jordifierro Thank you very much for your detailed explanation. I had tried to play around with Dagger 2 and the this framework these days and I think I understand your point for annotating them in a better way, though I might not fully grasp it. Anyway, I believe I will understand it better as I am diving deeper with my project using this framework, which greatly helped me so much in building the new application. Thank you again. I will keep you posted.

[jordifierro]

I am glad that this project is helping yours, and also appreciate your feedback and questions @TonyTangAndroid. Thanks!

[TonyTangAndroid]

Hey Jordi, I have found a better way to address the singleton annotation. Basically, as we have discussed, the annotation of Singleton could not guarantee that it will be singleton. It will be bounded together with object graph that dagger builds.

I have watched a video in youtube, which introduces a new customized scope called as ApplicationScope. If you truly expect the singleton of certain object, annotate with ApplicationScope instead of Singleton. Then you bind the ApplicationScope with ApplicationComponent. In this case, the singleton instance will be guaranteed as one Android application will only have one application component.

In this case, we could eliminate the singleton annotation in the application when expected and remove singleton annotation when it could not be guaranteed like in repository. So there will be no more confusion or wrong expectation.

For more details about what changes have to be done to use customized application scope, please refer to this comparison

https://github.com/TonyTangAndroid/android-base/compare/singleton_scope...TonyTangAndroid:application_scope