RecyclerView Reload Same Data When Refresh

I have a problem, when i swipe to refresh the data, the first swipe is ok but after that every swipe reload and add the same data over and over again, by the end i have a list with

Solution 1:

You are using ViewHolder#setIsRecyclable incorrectly; this method is meant to be used to prevent a ViewHolder from being recycled only while changes are being made to it. According to the documentation:

Calls to setIsRecyclable() should always be paired (one call to setIsRecyclabe(false) should always be matched with a later call to setIsRecyclable(true)).

This means none of your ViewHolder objects will be recycled, effectively making the use of a RecyclerView worthless, and preventing it from reusing the views when you attempt to bind new objects to your RecyclerView.

So, in short, remove that line of code.

---

I noticed a few other small issues with your adapter code as well, which can cause a multitude headaches in the future; so I took the liberty of highlighting some of the changes I would make.

Just for my own sanity, I will refer to your Articles class as Article.

It is usually not a good idea to pass around your Context all over the place. The View passed to your ViewHolder already has a reference to a Context, so you can use that instead.

As for the insertArticle() code, the Activity should be handling this anyway. So you can pass the Article back to the Activity by passing a listener to your Adapter (and subsequently, each ViewHolder) instead of the Context.

You should also consider using the DiffUtil class instead of just calling notifyDataSetChanged(); it is much more efficient. Just make sure your Article class is implementing equals() and hashCode() or it will not work.

I didn't include the animation code (that can easily be added back in) or the saved state code (mostly because I don't know what you were trying to do).

public class ArticleAdapter extends RecyclerView.Adapter<Article> {

    private List<Article> mData;

    private ArticleViewHolder.OnSelectedListener mOnSelectedListener;
    private ArticleViewHolder.OnSaveListener mOnSaveListener;

    public ArticleAdapter(ArticleViewHolder.OnSelectedListener onSelectedListener, ArticleViewHolder.OnSaveListener onSaveListener) {
        mOnSelectedListener = onSelectedListener;
        mOnSaveListener = onSaveListener;
        mData = new ArrayList<>();
    }

    public void replaceData(final List<Article> data) {
        final List<Article> oldData = new ArrayList<>(mData);
        mData.clear();

        if (data != null) {
            mData.addAll(data);
        }

        DiffUtil.calculateDiff(new DiffUtil.Callback() {
            @Override
            public int getOldListSize() {
                return oldData.size();
            }

            @Override
            public int getNewListSize() {
                return mData.size();
            }

            @Override
            public int areItemsTheSame(int oldItemPosition, int newItemPosition) {
                return oldData.get(oldItemPosition).equals(mData.get(newItemPosition));
            }

            @Override
            public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) {
                return oldData.get(oldItemPosition).equals(mData.get(newItemPosition));
            }
        }).dispatchUpdatesTo(this);
    }

    @Override
    public ArticleViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        View view = LayoutInflater.from(parent.getContext()).inflate(R.layout.item_card_view, parent, false);
        return new SelectLocationViewHolder(view, mOnSelectedListener, mOnSaveListener);
    }

    @Override
    public void onBindViewHolder(ArticleViewHolder holder, int position) {
        holder.bind(mData.get(position));
    }

    @Override
    public int getItemCount() {
        return mData.size();
    }

}

---

public class ArticleViewHolder extends RecyclerView.ViewHolder {

    public interface OnSelectedListener {
        void onSelected(Article article);
    }

    public interface OnSaveListener {
        void onSave(Article article);
    }

    private View mView;
    private Article mArticle;

    private OnSelectedListener mOnSelectedListener;
    private OnSaveListener mOnSaveListener;

    private ImageView mImageView;
    private TextView mTitleTextView, mDescriptionTextView;
    private FloatingActionButton mSaveButton;

    public ArticleViewHolder(View itemView, final OnSelectedListener onSelectedListener, final OnSaveListener onSaveListener) {
        super(itemView);

        mImageView = (ImageView) itemView.findViewById(R.id.photoImageView);
        mTitleTextView = (TextView) itemView.findViewById(R.id.titleWithoutImage);
        mDescriptionTextView = (TextView) itemView.findViewById(R.id.descriptionTextView);
        mSaveButton = (FloatingActionButton) itemView.findViewById(R.id.floatingActionButton);

        mView = itemView;
        mView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                onSelectedListener.onSelected(mArticle);
            }
        });

        mSaveButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                onSaveListener.onSave(mArticle);
            }
        });
    }

    public void bind(Article article) {
        mArticle = article;
        mTitleTextView.setText(article.getTitle());

        mDescriptionTextView.setText(article.getDescription());
        if(TextUtils.isEmpty(article.getDescription())) {
            mDescriptionTextView.setVisibility(View.GONE);
        }

        Glide.with(mView.getContext()).load(article.getImage()).into(mImageView);
    }

}

---

Edit

The actual issue is that your loader uses the same ArrayList every time, and keeps adding the new results to it.

public class NewsLoader extends AsyncTaskLoader<List<Article>> {

    private final String[] mUrls;
    private final OkHttpClient mClient;

    public NewsLoader(Context context, OkHttpClient client, String... urls) {
        super(context);
        mClient = client;
        mUrls = urls;
    }

    @Override
    public List<Article> loadInBackground() {
        List<Article> articles = new ArrayList<>();

        for (String url : mUrls) {
            Request request = new Request.Builder().url(url).build();
            try {
                Response response = mClient.newCall(request).execute();
                if (response.isSuccessful()) {
                    parseData(response.body().string(), articles);
                }
            } catch (IOException | JSONException e) {
                e.printStackTrace();
            }
        }

         return articles;
    }

    private void parseData(List<Article> articles, String data) throws JSONException {
        JSONObject forecast = new JSONObject(data);
        JSONArray a = forecast.getJSONArray("articles");

        for (int i = 0; i < a.length(); i++) {
            JSONObject o = a.getJSONObject(i);
            Article article = new Article(
                    o.getString("title"),
                    o.getString("description"),
                    o.getString("url"),
                    o.getString("urlToImage"));
            articles.add(article);
        }
    }

}

Also, you may have noticed, I made a small change to your Article constructor. You should consider making the Article class immutable, as this will prevent you from making mistakes when dealing with multithreading. It should look something like this:

public class Article {

    private final String mTitle;
    private final String mDescription;
    private final String mUrl;
    private final String mImageUrl;


    public Article(String title, String description, String url, String imageUrl) {
        mTitle = title;
        mDescription = description;
        mUrl = url;
        mImageUrl = imageUrl;
    }

    public String title() {
        return mTitle;
    }

    public String description() {
        return mDescription;
    }

    public String url() {
        return mUrl;
    }

    public String imageUrl() {
        return mImageUrl;
    }


    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        Article other = (Article) o;

        return mTitle != null && mTitle.equals(other.mTitle) &&
                mDescription != null && mDescription.equals(other.mDescription) &&
                mUrl != null && mUrl.equals(other.mUrl) &&
                mImageUrl != null && mImageUrl.equals(other.mImageUrl);
    }

    @Override
    public int hashCode() {
        int result = mTitle != null ? mTitle.hashCode() : 0;
        result = 31 * result + (mDescription != null ? mDescription.hashCode() : 0);
        result = 31 * result + (mUrl != null ? mUrl.hashCode() : 0);
        result = 31 * result + (mImageUrl != null ? mImageUrl.hashCode() : 0);
        return result;
    }

}

Solution 2:

@Override
public void onBindViewHolder(ArticleViewHolder holder, int position) {
    holder.bindArticle(mArticlesList.get(position));
    setAnimation(holder.itemView, position);
}


public void addAll(ArrayList<Articles> articles) {
    mArticlesList.clear();
    mArticlesList.addAll(articles);
    notifyDataSetChanged();
}

If this doesn't wrok then I think your api is giving you redundant data. Why you are using articleViewHolder.setIsRecyclable(false);

One another place which might cause the problem is

private void getMultipleUrls(String jsonData) throws JSONException {

    if (mArticlesArrayList == null) {
         mArticlesArrayList = getArticleForecast(jsonData);
    } else {
        mArticlesArrayList.addAll(getArticleForecast(jsonData));
    }
}

You are calling it from a loop add adding data to your arraylist. There somehow multiple data can be inserted in your ArrayList