Is "inject everything" a bad practice in Android?

Question

Studying about dependency injection I found some approaches that suggests to inject everything and other saying that it's not necessary to do so.

In my current project, my rule of thumb regarding Dependency Injection is "if the class was created by me, I make it injectable". In other words only classes like SimpleDateFormat, ArrayList, HashMap are newables in my project. My intent doing this approach is that I can @Inject any class anywhere once calling Injector.getApplicationComponent().inject(this) in the Activity. Basically all my classes have a non-args constructor with @Inject.

I was primary using DI because I thought it will improve the performance and memory usage once the new operator is used exclusively by the Dagger generated classes. But I read a post from Dagger 1 developer saying that DI does not have impact on performance and the usage is basically to reduce boilerplate.

The first question is:

• Dagger 2 does not any performance advantage in Android application?

My project is running without problems and I think the approach of "inject everything" helps organizing better, despite some drawbacks.

An example of usage of this approach is the following class:

public class TimelineEntryAdapter {

@Inject
Provider<TwitterEntry> mTwitterProvider;

@Inject
Provider<InstagramEntry> mInstagramProvider;

@Inject
Provider<FacebookEntry> mFacebookProvider;

@Inject
TimelineEntryComparator mComparator;

@Inject
public TimelineEntryAdapter() {
}

The second question is:

• Is it a bad practice to inject everything in Android?

If the answer for the second question is "No", there is a better way to handle the non-args constructor to create classes? Because when I create an non-args constructor with @Inject annotation and a class need some parameters to work with, I must use setters:

public class SavelArtist {

private MusicBrainzArtist mMusicBrainzArtist;

private DiscogsArtist mDiscogsArtist;

private List<SavelTweet> mTweetList;

private SpotifyArtist mSpotifyArtist;

private List<SavelInstagram> mInstaTimeline;

private List<SavelFacebook> mFacebookTimeline;

private List<SavelRelease> mReleases;

@Inject
Provider<SavelRelease> mReleaseProvider;

@Inject
public SavelArtist() {
}

public void setMusicBrainzArtist(MusicBrainzArtist mbArtist) {
    mMusicBrainzArtist = mbArtist;
}

public void setDiscogsArtist(DiscogsArtist discogsArtist) {
    mDiscogsArtist = discogsArtist;
}

public void setTweetList(List<SavelTweet> tweetList) {
    mTweetList = tweetList;
}

public void setSpotifyArtist(SpotifyArtist spotifyArtist) {
    mSpotifyArtist = spotifyArtist;
}

public void setInstaTimeline(List<SavelInstagram> instaTimeline) {
    mInstaTimeline = instaTimeline;
}

public void setFacebookTimeline(List<SavelFacebook> fbTimeline) {
    mFacebookTimeline = fbTimeline;
}

All the parameters could be set on the constructor, once all are get at the same time in the flow.

Answer 1

Studying about dependency injection I found some approaches that suggests to inject everything and other saying that it's not necessary to do so.

The froger_mcs blog entry you quote doesn't advocate injecting everything. It quite clearly states:

The purpose of this post is to show what we can do, not what we should do.

And it goes on to state a disadvantage of injecting everything:

If you want to use Dagger 2 for almost everything in your project you will quickly see that big piece of 64k methods count limit is used by generated code for injections.

Now, on to your questions:

Dagger 2 does not any performance advantage in Android application?

While Dagger 2 offers a performance advantage over other reflection-based DI frameworks (such as Guice) it doesn't claim to offer any performance advantage over manually constructing your object graphs by calling constructors. You can inspect the generated classes yourself to see that these indeed still eventually call constructors.

Is it a bad practice to inject everything in Android?

Well let's take the following very common Android code:

Intent nextActivity = new Intent(this, NextActivity.class);
startActivity(nextActivity);

Should we extract an IntentFactory and inject this using Dagger 2 merely to avoid the new keyword here? At this point, it is easy to approach pedantry. The advice in the other answer you quoted about the difference between injectables and newables is more flexible and elegant.

Moving on to your next question:

If the answer for the second question is "No", there is a better way to handle the non-args constructor to create classes? Because when I create an non-args constructor with @Inject annotation and a class need some parameters to work with, I must use setters:

Using setters is the wrong approach for parameters. You should distinguish dependencies and parameters. Dependencies normally have the same lifecycle as the object itself. During the lifetime of the object, the methods exposed for that object may be called with different parameters.

Answer 2

I'm not a professional at dependency-management but I use it in my company so here's how I see it.

Inject everything is basically good. I make everything that is used somewhere else ( other modules, classes, packages ) injectable and only static things ( static classes with hidden constructor ), things that get only used internally non injectable.

What's the advantage of it? Well The dependency system will take care of getting the instance and to discard it. It will clean up automatically. Also using scopes on the classes will enable you to make a class that always has only one instance in the whole application ( multiple threads access it) or make it reusable ( every thread that needs it get's an own instance ).

Also I think you could clean up your classes like this:

@Reusable (Maybe a good Idea?)
public class SavelArtist {

private MusicBrainzArtist mMusicBrainzArtist;

private DiscogsArtist mDiscogsArtist;

private List<SavelTweet> mTweetList;

private SpotifyArtist mSpotifyArtist;

private List<SavelInstagram> mInstaTimeline;

private List<SavelFacebook> mFacebookTimeline;

private List<SavelRelease> mReleases;

private Provider<SavelRelease> mReleaseProvider;

public SavelArtist() {
}

@Inject
public void init(Provider<SavelRelease> mReleaseProvider) {
  this.mReleaseProvider = mReleaseProvider;
}

Think about always declaring the scope of the class. Is it a Singleton? Will it be reusable? This little detail can save you, once the application get's complex and big.

The advantage of using the method init to declare all injected variables is to have a clean looking code which is easy maintainable since everything injected is at this one location. But that's actually preference :)