ant2009
ant2009

Reputation: 22696

How to inject an Activity into an Adapter using dagger2

Android Studio 3.0 Canary 8

I am trying to inject my MainActivity into my Adapter. However, my solution works ok, but I think its a code smell and not the right way to do it.

My adapter snippet looks like this the but I don't like about this is that I have to cast the Activity to MainActivity:

public class RecipeAdapter extends RecyclerView.Adapter<RecipeListViewHolder> {
    private List<Recipe> recipeList = Collections.emptyList();
    private Map<Integer, RecipeListViewHolderFactory> viewHolderFactories;
    private MainActivity mainActivity;

    public RecipeAdapter(Activity activity, Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
        this.recipeList = new ArrayList<>();
        this.viewHolderFactories = viewHolderFactories;
        this.mainActivity = (MainActivity)activity;
    }

    @Override
    public RecipeListViewHolder onCreateViewHolder(ViewGroup viewGroup, int i) {
        /* Inject the viewholder */
        final RecipeListViewHolder recipeListViewHolder = viewHolderFactories.get(Constants.RECIPE_LIST).createViewHolder(viewGroup);

        recipeListViewHolder.itemView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                /* Using the MainActivity to call a callback listener */
                mainActivity.onRecipeItemClick(getRecipe(recipeListViewHolder.getAdapterPosition()));
            }
        });

        return recipeListViewHolder;
    }
}

In my Module, I pass the Activity in the module's constructor and pass it to the Adapter.

@Module
public class RecipeListModule {
    private Activity activity;

    public RecipeListModule() {}

    public RecipeListModule(Activity activity) {
        this.activity = activity;
    }

    @RecipeListScope
    @Provides
    RecipeAdapter providesRecipeAdapter(Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
        return new RecipeAdapter(activity, viewHolderFactories);
    }
}

In My Application class I create the components and I am using a SubComponent for the adapter. Here I have to pass the Activity which I am not sure is a good idea.

@Override
public void onCreate() {
    super.onCreate();

    applicationComponent = createApplicationComponent();
    recipeListComponent = createRecipeListComponent();
}

public BusbyBakingComponent createApplicationComponent() {
    return DaggerBusbyBakingComponent.builder()
            .networkModule(new NetworkModule())
            .androidModule(new AndroidModule(BusbyBakingApplication.this))
            .exoPlayerModule(new ExoPlayerModule())
            .build();
}

public RecipeListComponent createRecipeListComponent(Activity activity) {
    return recipeListComponent = applicationComponent.add(new RecipeListModule(activity));
}

My Fragment I inject like this:

@Inject RecipeAdapter recipeAdapter;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        ((BusbyBakingApplication)getActivity().getApplication())
                .createRecipeListComponent(getActivity())
                .inject(this);
    }

Even though the above design works, I think it's a code smell as I have to cast the Activity to the MainActivity. The reason I use the Activity as I want to make this module more generic.

Just wondering if there is a better way

=============== UPDATE USING INTERFACE

Interface

public interface RecipeItemClickListener {
    void onRecipeItemClick(Recipe recipe);
}

Implementation

public class RecipeItemClickListenerImp implements RecipeItemClickListener {
    @Override
    public void onRecipeItemClick(Recipe recipe, Context context) {
        final Intent intent = Henson.with(context)
                .gotoRecipeDetailActivity()
                .recipe(recipe)
                .build();

        context.startActivity(intent);
    }
}

In my module, I have the following providers

@Module
public class RecipeListModule {
    @RecipeListScope
    @Provides
    RecipeItemClickListener providesRecipeItemClickListenerImp() {
        return new RecipeItemClickListenerImp();
    }

    @RecipeListScope
    @Provides
    RecipeAdapter providesRecipeAdapter(RecipeItemClickListener recipeItemClickListener, Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
        return new RecipeAdapter(recipeItemClickListener, viewHolderFactories);
    }
}

Then I use it through constructor injection in the RecipeAdapter

public class RecipeAdapter extends RecyclerView.Adapter<RecipeListViewHolder> {
    private List<Recipe> recipeList = Collections.emptyList();
    private Map<Integer, RecipeListViewHolderFactory> viewHolderFactories;
    private RecipeItemClickListener recipeItemClickListener;

    @Inject /* IS THIS NESSESSARY - AS IT WORKS WITH AND WITHOUT THE @Inject annotation */
    public RecipeAdapter(RecipeItemClickListener recipeItemClickListener, Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
        this.recipeList = new ArrayList<>();
        this.viewHolderFactories = viewHolderFactories;
        this.recipeItemClickListener = recipeItemClickListener;
    }

    @Override
    public RecipeListViewHolder onCreateViewHolder(final ViewGroup viewGroup, int i) {
        /* Inject the viewholder */
        final RecipeListViewHolder recipeListViewHolder = viewHolderFactories.get(Constants.RECIPE_LIST).createViewHolder(viewGroup);

        recipeListViewHolder.itemView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                recipeItemClickListener.onRecipeItemClick(getRecipe(recipeListViewHolder.getAdapterPosition()), viewGroup.getContext());
            }
        });

        return recipeListViewHolder;
    }
}

Just one question, is the @Inject annotation need for the constructor in the RecipeAdapter. As it works with or without the @Inject.

Upvotes: 8

Views: 6049

Answers (3)

David Medenjak
David Medenjak

Reputation: 34542

If you need a MainActivity then you should also provide it. Instead of Activity declare MainActivity for your module.

@Module
public class RecipeListModule {
  private MainActivity activity;

  public RecipeListModule(MainActivity activity) {
    this.activity = activity;
  }
}

And your Adapter should just request it (Constructor Injection for non Android Framework types!)

@RecipeListScope
class RecipeAdapter {

  @Inject
  RecipeAdapter(MainActivity activity,
          Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
    // ...
  }

}

If you want your module to use Activity and not MainActivity then you will need to declare an interface as already mentioned. You adapter would then declare the interface as its dependency.

But in some module you will still have to bind that interface to your MainActivity and one module needs to know how to provide the dependency.

// in some abstract module
@Binds MyAdapterInterface(MainActivity activity) // bind the activity to the interface

Addressing the updated part of the question

Just one question, is the @Inject annotation need for the constructor in the RecipeAdapter. As it works with or without the @Inject.

It works without it because you're still not using constructor injection. You're still calling the constructor yourself in providesRecipeAdapter(). As a general rule of thumb—if you want to use Dagger properly—don't ever call new yourself. If you want to use new ask yourself if you could be using constructor injection instead.

The same module you show could be written as follows, making use of @Binds to bind an implementation to the interface, and actually using constructor injection to create the adapter (which is why we don't have to write any method for it! Less code to maintain, less errors, more readable classes)

As you see I don't need to use new myself—Dagger will create the objects for me.

public abstract class RecipeListModule {
  @RecipeListScope
  @Binds
  RecipeItemClickListener providesRecipeClickListener(RecipeItemClickListenerImp listener);
}

Upvotes: 8

Yossi Segev
Yossi Segev

Reputation: 607

Do not pass Activities into Adapters - This is a really bad practice.

Inject only the fields you care about.

In your example: Pass an interface into the adapter to track the item click.

Upvotes: 14

EpicPandaForce
EpicPandaForce

Reputation: 81588

Personally I would do the following trick

public class MainActivity extends AppCompatActivity {
    private static final String TAG = "__ACTIVITY__";

    public static MainActivity get(Context context) {
        // noinspection ResourceType
        return (MainActivity)context.getSystemService(TAG);
    }

    @Override
    protected Object getSystemService(String name) {
        if(TAG.equals(name)) {
            return this;
        }
        return super.getSystemService(name);
    }
}

public class RecipeAdapter extends RecyclerView.Adapter<RecipeListViewHolder> {
    private List<Recipe> recipeList = Collections.emptyList();
    private Map<Integer, RecipeListViewHolderFactory> viewHolderFactories;

    public RecipeAdapter(Map<Integer, RecipeListViewHolderFactory> viewHolderFactories) {
        this.recipeList = new ArrayList<>();
        this.viewHolderFactories = viewHolderFactories;
    }

    @Override
    public RecipeListViewHolder onCreateViewHolder(ViewGroup viewGroup, int i) {
        /* Inject the viewholder */
        final RecipeListViewHolder recipeListViewHolder = viewHolderFactories.get(Constants.RECIPE_LIST).createViewHolder(viewGroup);

        recipeListViewHolder.itemView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                MainActivity mainActivity = MainActivity.get(v.getContext());
                if(recipeListViewHolder.getAdapterPosition() != -1) {
                    mainActivity.onRecipeItemClick(
                      getRecipe(recipeListViewHolder.getAdapterPosition()));
                }
            }
        });

        return recipeListViewHolder;
    }
}

Upvotes: 2

Related Questions