Skip to content
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

[About][Dagger] ❓Possibly redundant @Inject annotation on AboutStyler constructor #832

Open
geaden opened this issue May 15, 2020 · 3 comments

Comments

@geaden
Copy link

geaden commented May 15, 2020

I noticed, that AboutStyler constructor has @Inject annotation, however at the same time AboutStyler is provided via @Provides from AboutActivityModule with @FeatureScope. Having looked at generated by Dagger source code, AboutStyler_Factory is not used anywhere and DaggerAboutComponent uses AboutActivityModule_ProvideAboutStylerFactory instead.

I removed @Inject annotation from AboutStyler and run ./gradlew :about:assembleDebug --profile. It seems everything is working, DaggerAboutComponent remained the same, but AboutStyler_Factory is not generated. Rough measurement on clean about module on my machine shew, that task :about:kaptDebugKotlin took 2.686s with @Inject and 2.299s without.

Moreover AboutActivity is already bounded in AboutComponent.Builder, so one can remove provideContext and provideAboutStyler from AboutActivityModule, keeping @Inject on AboutStyler. I also added @FeatureScope to AboutStyler class. This time generated AboutStyler_Factory is used by DaggerAboutComponent.

Diff

--- about/build/generated/source/kapt/debug/io/plaidapp/about/dagger/DaggerAboutComponent.java  2020-05-15 22:03:28.000000000 +0300
+++ about/build/generated/source/kapt/debug/io/plaidapp/about/dagger/DaggerAboutComponent.java  2020-05-15 22:03:34.000000000 +0300
@@ -2,11 +2,13 @@
 package io.plaidapp.about.dagger;
 
 import dagger.internal.DoubleCheck;
+import dagger.internal.InstanceFactory;
 import dagger.internal.Preconditions;
 import in.uncod.android.bypass.Markdown;
 import io.plaidapp.about.ui.AboutActivity;
 import io.plaidapp.about.ui.AboutActivity_MembersInjector;
 import io.plaidapp.about.ui.AboutStyler;
+import io.plaidapp.about.ui.AboutStyler_Factory;
 import io.plaidapp.about.ui.model.AboutViewModelFactory;
 import io.plaidapp.core.dagger.MarkdownModule;
 import io.plaidapp.core.dagger.MarkdownModule_ProvideMarkdownFactory;
@@ -19,14 +21,16 @@
 public final class DaggerAboutComponent implements AboutComponent {
   private final AboutActivityModule aboutActivityModule;
 
-  private Provider<AboutStyler> provideAboutStylerProvider;
+  private Provider<AboutActivity> activityProvider;
+
+  private Provider<AboutStyler> aboutStylerProvider;
 
   private Provider<Markdown> provideMarkdownProvider;
 
   private DaggerAboutComponent(AboutActivityModule aboutActivityModuleParam,
-      MarkdownModule markdownModuleParam, AboutActivity activity) {
+      MarkdownModule markdownModuleParam, AboutActivity activityParam) {
     this.aboutActivityModule = aboutActivityModuleParam;
-    initialize(aboutActivityModuleParam, markdownModuleParam, activity);
+    initialize(aboutActivityModuleParam, markdownModuleParam, activityParam);
   }
 
   public static AboutComponent.Builder builder() {
@@ -34,12 +38,13 @@
   }
 
   private AboutViewModelFactory getAboutViewModelFactory() {
-    return new AboutViewModelFactory(provideAboutStylerProvider.get(), AboutActivityModule_ProvideResourcesFactory.provideResources(aboutActivityModule), provideMarkdownProvider.get());}
+    return new AboutViewModelFactory(aboutStylerProvider.get(), AboutActivityModule_ProvideResourcesFactory.provideResources(aboutActivityModule), provideMarkdownProvider.get());}
 
   @SuppressWarnings("unchecked")
   private void initialize(final AboutActivityModule aboutActivityModuleParam,
-      final MarkdownModule markdownModuleParam, final AboutActivity activity) {
-    this.provideAboutStylerProvider = DoubleCheck.provider(AboutActivityModule_ProvideAboutStylerFactory.create(aboutActivityModuleParam));
+      final MarkdownModule markdownModuleParam, final AboutActivity activityParam) {
+    this.activityProvider = InstanceFactory.create(activityParam);
+    this.aboutStylerProvider = DoubleCheck.provider(AboutStyler_Factory.create(activityProvider));
     this.provideMarkdownProvider = DoubleCheck.provider(MarkdownModule_ProvideMarkdownFactory.create(markdownModuleParam));
   }

The question is: is it safe to remove @Inject annotation from AboutStyler or it's better to remove provide* methods from AboutActivityModule and keep @Inject annotation on AboutStyler constructor? Or we have to keep everything as it is, but we'll have generated unused AboutStyler_Factory. This should not be a problem since R8/Proguard will strip such classes, but I suppose it might decrease build time during development, if there are more such cases.

I can make all required changes if needed.

Thanks.

@geaden
Copy link
Author

geaden commented May 19, 2020

I found this 3 years old article by @pyricau where @Inject is favored over @Provides. Probably, this should be taken into consideration.

@pyricau
Copy link

pyricau commented May 19, 2020

You can safely remove the @Inject, unfortunately Dagger doesn't error out when there are two ways to provide a binding. Creating AboutStyler requires an activity instance, which isnt available in the graph and is passed in as a module param.

@geaden
Copy link
Author

geaden commented May 19, 2020

You can safely remove the @Inject, unfortunately Dagger doesn't error out when there are two ways to provide a binding. Creating AboutStyler requires an activity instance, which isnt available in the graph and is passed in as a module param.

Great! Thanks for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants