Skip to content

Commit

Permalink
[ MVP ] Fix some DI issues: different instances; presenter in adapter…
Browse files Browse the repository at this point in the history
… is null
  • Loading branch information
li2 committed May 16, 2018
1 parent f08b0cd commit 42ec53f
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 150 deletions.
Expand Up @@ -13,6 +13,9 @@
import dagger.Module;
import dagger.Provides;
import me.li2.android.architecture.data.model.Article;
import me.li2.android.architecture.data.source.remote.ArticleDeserializer;
import me.li2.android.architecture.data.source.remote.ArticleListDeserializer;
import me.li2.android.architecture.data.source.remote.DemoWebService;
import me.li2.android.architecture.utils.NetworkConnectivityInterceptor;
import okhttp3.OkHttpClient;
import okhttp3.logging.HttpLoggingInterceptor;
Expand Down
Expand Up @@ -7,10 +7,12 @@
import me.li2.android.architecture.ui.list.ArticleListActivity;
import me.li2.android.architecture.ui.list.ArticleListFragment;
import me.li2.android.architecture.ui.list.ArticleListFragmentModule;
import me.li2.android.architecture.ui.list.ArticlesScope;

/**
* This is a given module to dagger.
* We map ALL our activities here, then Dagger knows our activities in compile time.
* Otherwise it causes IllegalArgumentException: No injector factory bound for Class
*
* @author Weiyi Li on 31/3/18 | https://github.com/li2
*/
Expand All @@ -21,22 +23,10 @@ public abstract class ActivityBuilder {
@ContributesAndroidInjector(modules = ArticleDetailActivityModule.class)
abstract ArticleDetailActivity bindArticleDetailActivity();

/**
* ArticleListFragment is hosted by ArticleListActivity, so we need to
* install the fragment's module to this activity.
*/
@ContributesAndroidInjector(modules = { ArticleListActivityHostedFragment.class })
@ContributesAndroidInjector()
abstract ArticleListActivity bindArticleListActivity();

/**
* The reason to create this class is that this activity maybe host many fragments.
*/
@Module
abstract class ArticleListActivityHostedFragment {

@ContributesAndroidInjector(modules = ArticleListFragmentModule.class)
abstract ArticleListFragment bindArticleListFragment();

// another fragment ...
}
@ContributesAndroidInjector(modules = ArticleListFragmentModule.class)
@ArticlesScope
abstract ArticleListFragment bindArticleListFragment();
}
21 changes: 21 additions & 0 deletions app/src/main/java/me/li2/android/architecture/di/AppModule.java
Expand Up @@ -4,6 +4,9 @@
import android.arch.persistence.room.Room;
import android.content.Context;

import com.jakewharton.picasso.OkHttp3Downloader;
import com.squareup.picasso.Picasso;

import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -49,6 +52,7 @@ DemoDatabase provideDemoDatabase(Context context) {
}

@Provides
@Singleton
ArticleDao provideArticleDao(DemoDatabase database) {
return database.articleDao();
}
Expand All @@ -57,4 +61,21 @@ ArticleDao provideArticleDao(DemoDatabase database) {
RateLimiter<String> provideRateLimiter() {
return new RateLimiter<>(2, TimeUnit.MINUTES);
}


@Provides
@Singleton
OkHttp3Downloader provideOkHttp3Downloader(Context context) {
return new OkHttp3Downloader(context.getApplicationContext(), Integer.MAX_VALUE);
}

@Provides
@Singleton
Picasso providePicasso(Context context, OkHttp3Downloader downloader) {
return new Picasso.Builder(context.getApplicationContext())
.loggingEnabled(true)
.downloader(downloader)
.listener((picasso1, uri, exception) -> exception.printStackTrace())
.build();
}
}
@@ -1,6 +1,5 @@
package me.li2.android.architecture.ui.list;

import android.app.ActivityOptions;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
Expand All @@ -25,21 +24,24 @@
import me.li2.android.architecture.ui.detail.ArticleDetailActivity;
import me.li2.android.architecture.utils.NetworkUtils;

public class ArticleListFragment extends DaggerFragment implements ArticleSelectListener, ArticlesContract.View {
public class ArticleListFragment extends DaggerFragment implements ArticlesContract.View {

private static final String LOG_TAG = ArticleListFragment.class.getSimpleName();

private static final String BUNDLE_RECYCLER_POSITION = "recycler_position";

@BindView(R.id.article_list_view)
RecyclerView mRecyclerView;

@BindView(R.id.article_list_swiperefresh)
SwipeRefreshLayout mSwipeRefreshLayout;

@Inject
ArticlesContract.Presenter mPresenter;

@Inject
ArticlesAdapter mAdapter;

@BindView(R.id.article_list_view)
RecyclerView mRecyclerView;

@BindView(R.id.article_list_swiperefresh)
SwipeRefreshLayout mSwipeRefreshLayout;

public ArticleListFragment() {
// Required empty public constructor
Expand Down Expand Up @@ -102,7 +104,7 @@ public void onPause() {
private SwipeRefreshLayout.OnRefreshListener mOnRefreshListener = new SwipeRefreshLayout.OnRefreshListener() {
@Override
public void onRefresh() {
mPresenter.loadArticles();
mPresenter.onUserRefresh();
}
};

Expand All @@ -120,83 +122,31 @@ private void showMessage(int stringResId) {
.show();
}

// Delegate ViewHolder click event to Fragment, then we can use activity to implement transition animation.
@Override
public void onArticleSelect(Article article, View sharedView, String sharedName) {
ActivityOptions options = ActivityOptions.makeSceneTransitionAnimation(getActivity(), sharedView, sharedName);
startActivity(ArticleDetailActivity.newIntent(getContext(), article.getId()), options.toBundle());
}

@Override
public void showArticles() {
mAdapter.notifyDataSetChanged();
}

@Override
public void setLoadingIndicator(boolean active) {
mSwipeRefreshLayout.setRefreshing(active);
}

@Override
public void showLoadingArticlesSucceed() {
showMessage(R.string.status_success);
public void showArticleList() {
mAdapter.notifyDataSetChanged();
}

@Override
public void showNoArticles() {
public void showNoArticlesView() {
showMessage(R.string.status_no_response);
}

@Override
public void showLoadingArticlesError() {

}

@Override
public void showNoNetworkError() {
showMessage(R.string.status_no_connect);
}

/*
The reason to make Adapter as an inner class is to use Presenter easily.
To use MVP for RecyclerView,
Normally we create a Collection (let it be a List) field within the adapter, holding all the data that it needs to display.
This sucks, because in MVP we typically manager data in presenter,
this makes the list to be referenced (or worse: copied) in two different places,
which doubles our effort to keep those two in sync when making changes.
Instead, we should use presenter !
https://android.jlelse.eu/recyclerview-in-mvp-passive-views-approach-8dd74633158
*/

public class ArticlesAdapter extends RecyclerView.Adapter<ArticleViewHolder> {

private Context mContext;
private ArticleSelectListener mArticleSelectListener;

//error: @Inject constructors are invalid on inner classes
//@Inject
public ArticlesAdapter(Context context, ArticleSelectListener listener) {
mContext = context;
mArticleSelectListener = listener;
}

@Override
public int getItemCount() {
return mPresenter.getArticlesCount();
}

@Override
public ArticleViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
View view = LayoutInflater.from(mContext).inflate(R.layout.article_list_view_holder, parent, false);
return new ArticleViewHolder(view);
}

@Override
public void onBindViewHolder(ArticleViewHolder holder, int position) {
holder.bindArticle(mPresenter.getArticle(position), mArticleSelectListener);
}
@Override
public void showArticleDetailView(Article article) {
// activity transition animation
// ActivityOptions options = ActivityOptions.makeSceneTransitionAnimation(getActivity(), sharedView, sharedName);
// startActivity(ArticleDetailActivity.newIntent(getContext(), article.getId()), options.toBundle());
startActivity(ArticleDetailActivity.newIntent(getContext(), article.getId()));
}
}
@@ -1,7 +1,6 @@
package me.li2.android.architecture.ui.list;

import android.arch.lifecycle.LifecycleOwner;
import android.content.Context;

import dagger.Module;
import dagger.Provides;
Expand All @@ -14,19 +13,14 @@

@Module
public class ArticleListFragmentModule {

@Provides
ArticleListFragment.ArticlesAdapter provideArticleListAdapter(Context context, ArticleListFragment fragment) {
return fragment.new ArticlesAdapter(context, fragment);
}

/*
error: ArticlesContract.Presenter cannot be provided without an @Provides- or @Produces-annotated method.
the reason is that we do this in fragment, however, this is a interface which cannot use @Inject.
@Inject
ArticlesContract.Presenter mPresenter;
/**
* Provide dependency for interface.
* Interface cannot be annotated with @Inject, otherwise it will cause
* error: ArticlesContract.Presenter cannot be provided without an @Provides- or @Produces-annotated method.
*/
@Provides
@ArticlesScope
ArticlesContract.Presenter provideArticlesPresenter(ArticlesPresenter presenter) {
return presenter;
}
Expand Down

This file was deleted.

Expand Up @@ -10,6 +10,8 @@
import com.squareup.picasso.NetworkPolicy;
import com.squareup.picasso.Picasso;

import javax.inject.Inject;

import butterknife.BindView;
import butterknife.ButterKnife;
import me.li2.android.architecture.R;
Expand All @@ -35,22 +37,27 @@ public class ArticleViewHolder extends RecyclerView.ViewHolder {
@BindView(R.id.article_image_view)
ImageView mImageView;

public ArticleViewHolder(View itemView) {
@Inject
Picasso mPicasso;

private ArticlesContract.Presenter mPresenter;

public ArticleViewHolder(View itemView, ArticlesContract.Presenter presenter) {
super(itemView);
ButterKnife.bind(this, itemView);
mContext = itemView.getContext();
mItemView = itemView;
mPresenter = presenter;
}

public void bindArticle(Article article, ArticleSelectListener listener) {
public void bindArticle(Article article) {
if (article == null) {
return;
}
mTitleView.setText(article.getTitle());
mDescriptionView.setText(article.getDescription());
loadImage(mImageView, article.getImageHref());

mItemView.setOnClickListener(v -> listener.onArticleSelect(article, mImageView, mContext.getString(R.string.transition_article_image)));
mItemView.setOnClickListener(v -> mPresenter.onUserSelectArticle(article));
}

/**
Expand All @@ -66,7 +73,7 @@ public void bindArticle(Article article, ArticleSelectListener listener) {
private void loadImage(final ImageView imageView, final String imageHref) {
imageView.setVisibility(View.GONE);

Picasso.with(mContext)
mPicasso
.load(imageHref)
.networkPolicy(NetworkPolicy.OFFLINE) // force offline
.into(imageView, new Callback() {
Expand All @@ -77,7 +84,7 @@ public void onSuccess() {

@Override
public void onError() {
Picasso.with(mContext)
mPicasso
.load(imageHref)
.into(imageView, new Callback() {
@Override
Expand Down
@@ -0,0 +1,55 @@
package me.li2.android.architecture.ui.list;

import android.content.Context;
import android.support.v7.widget.RecyclerView;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;

import javax.inject.Inject;

import me.li2.android.architecture.R;

/**
*
To use MVP for RecyclerView,
Normally we create a Collection (let it be a List) field within the adapter, holding all the data that it needs to display.
This sucks, because in MVP we typically manager data in presenter,
this makes the list to be referenced (or worse: copied) in two different places,
which doubles our effort to keep those two in sync when making changes.
Instead, we should use presenter !
https://android.jlelse.eu/recyclerview-in-mvp-passive-views-approach-8dd74633158
* @author Weiyi Li on 6/5/18 | https://github.com/li2
*/

public class ArticlesAdapter extends RecyclerView.Adapter<ArticleViewHolder> {

private Context mContext;

@Inject
ArticlesContract.Presenter mPresenter;

@Inject
public ArticlesAdapter(Context context) {
mContext = context;
}

@Override
public int getItemCount() {
return mPresenter.getArticlesCount();
}

@Override
public ArticleViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
View view = LayoutInflater.from(mContext).inflate(R.layout.article_list_view_holder, parent, false);
return new ArticleViewHolder(view, mPresenter);
}

@Override
public void onBindViewHolder(ArticleViewHolder holder, int position) {
holder.bindArticle(mPresenter.getArticle(position));
}
}

0 comments on commit 42ec53f

Please sign in to comment.