plaid icon indicating copy to clipboard operation
plaid copied to clipboard

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

Open geaden opened this issue 4 years ago • 3 comments

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 avatar May 15 '20 19:05 geaden

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

geaden avatar May 19 '20 11:05 geaden

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.

pyricau avatar May 19 '20 14:05 pyricau

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!

geaden avatar May 19 '20 17:05 geaden