Abhinav Tripathi
Abhinav Tripathi

Reputation: 178

RecyclerView not scrolling smoothly

I have an app where I have several views(list item of different types) inside a RecyclerView. Each item is updated after fetching data from database.I am facing the issue of sticky scroll while scrolling through the items in my RecyclerView. I'm sure that this is happening due to the database access operation and I tried to address that. I referred to several answers on StackOverflow and also read through the official android post(Making ListView Scrolling Smooth). As suggested in the android post, I moved all my database access operations to a background thread using AsyncTask but still found no luck. Below is the code that I write inside onBindViewHolder to update the one of the items.

new AsyncTask<RecyclerView.ViewHolder, Void, TaskPhysicalActivity>(){
                private RecyclerView.ViewHolder v;
                PhysicalActivityItemHolder pAItemHolder;
                DBHelper mDbHelper;
                User mUser;
                @Override
                protected TaskPhysicalActivity doInBackground(RecyclerView.ViewHolder... params) {
                    pAItemHolder = (PhysicalActivityItemHolder)params[0];
                    Calendar cal = Calendar.getInstance();
                    Calendar today = Calendar.getInstance();
                    Calendar earlier = Calendar.getInstance();
                    earlier.add(Calendar.DATE, -6);
                    mDbHelper = DBHelper.getInstance(mContext);
                    mUser = User.getDefaultUser(mContext);
                    try {
                        List<PhysicalActivity> physicalActivities = mDbHelper.getPhysicalActivityInRange(mUser, cal, cal);
                        PhysicalActivityReport physicalActivityReport = new PhysicalActivityReport(earlier.getTime(), today.getTime());
                        physicalActivityReport.makeReport(mContext, mUser);
                        int userStepGoal = PhysicalActivity.getDailyStepsGoal(mContext);
                        return new TaskPhysicalActivity(userStepGoal, physicalActivityReport.getPhysicalActivityList(), physicalActivities,physicalActivityReport);
                    } catch (ParseException e) {
                        e.printStackTrace();
                    } catch (Exception e) {
                        e.printStackTrace();
                    }

                    return null;
                }


                @Override
                protected void onPostExecute(TaskPhysicalActivity taskPhysicalActivity) {
                    int stepsCount = 0;
                    Calendar cal = Calendar.getInstance();
                    TaskPhysicalActivity taskPhysicalActivityObject = taskPhysicalActivity;
                    List<PhysicalActivity> physicalActivities = taskPhysicalActivityObject.getPhysicalActivities();
                    List<PhysicalActivity> activityList = taskPhysicalActivityObject.getActivityList();
                    PhysicalActivityReport physicalActivityReport = taskPhysicalActivityObject.getPhysicalActivityReport();

                    pAItemHolder.tvActivityEmptyText.setVisibility(View.GONE);

                    if (physicalActivities.size() > 0) {
                        for (int i = 0; i < physicalActivities.size(); i++) {
                            stepsCount = stepsCount
                                    + physicalActivities.get(i).getValue();
                        }

                        final int finalStepsCount = stepsCount > 0 ? stepsCount : 0;
                        pAItemHolder.tvActivityValue.setText(String.valueOf(finalStepsCount));
                        pAItemHolder.tvActivityValueText.setText(mContext.getResources().getString(R.string.steps_unit));
                        pAItemHolder.tvActivityEmptyText.setText("");
                        pAItemHolder.vActivityCardEmptyView.setVisibility(View.GONE);
                        pAItemHolder.vActivityCardView.setVisibility(View.VISIBLE);

                    } else {

                        pAItemHolder.tvActivityValue.setText("--");
                        pAItemHolder.tvActivityValueText.setText(mContext.getResources().getString(R.string.steps_unit));
                        pAItemHolder.tvActivityEmptyText.setText("");
                        pAItemHolder.vActivityCardEmptyView.setVisibility(View.VISIBLE);
                        pAItemHolder.vActivityCardView.setVisibility(View.GONE);

                    }

                    pAItemHolder.tvActivityGoal.setText(String.format(mContext.getResources().getString(R.string.activity_goal_display_string),
                            taskPhysicalActivityObject.getUserStepsGoal(), mContext.getResources().getString(R.string.steps_unit), mContext.getResources().getString(R.string.per_day)));
                    pAItemHolder.vActivityGraph.setActivityValues(activityList);

                    if (physicalActivityReport.getPhysicalActivityAverage() > 0) {
                        pAItemHolder.tvPercentInRange.setText("Average steps " + physicalActivityReport.getPhysicalActivityAverage());
                    }
                    if (activityList.size() == 0) {
                        pAItemHolder.vActivityCardEmptyView.setVisibility(View.VISIBLE);
                        pAItemHolder.vActivityCardView.setVisibility(View.GONE);
                    } else {
                        pAItemHolder.vActivityCardEmptyView.setVisibility(View.GONE);
                        pAItemHolder.vActivityCardView.setVisibility(View.VISIBLE);
                    }
                }
            }.execute(holder);

In the code above, the following four lines inside doInBackground perform database access:

List<PhysicalActivity> physicalActivities = mDbHelper.getPhysicalActivityInRange(mUser, cal, cal);
PhysicalActivityReport physicalActivityReport = new PhysicalActivityReport(earlier.getTime(), today.getTime());
physicalActivityReport.makeReport(mContext, mUser);
int userStepGoal = PhysicalActivity.getDailyStepsGoal(mContext);

I use the results from these operations to update the UI in onPostExecute method.
Is there some issue with the way I have used AsyncTask ? Am I missing something?

Upvotes: 3

Views: 5398

Answers (2)

Abhinav Tripathi
Abhinav Tripathi

Reputation: 178

Thanks @Rafal and @r-zagórski for valuable suggestions. I tried the approach of keeping data ready and passing it to onBindViewHolder but for some reason that did not help. Then I moved all the code inside onBindViewHolder to onCreateViewHolder and pass an object to the adapter constructor with all required parameters already calculated and packaged in this object. So listing down the steps:

  1. Create a class(say DataClass) to hold all the required values and their getters to be used later in the recyclerView adapter during updates.
  2. Since database access is memory intensive, use an AsyncTask to perform all database access operations inside doInBackground method and make it return an object of DataClass.

     protected DataClass doInBackground(Params... params) {
     //perform database or other operations here
     // Let's say DataClass constructor takes in 3 parameters which are later used to update recycler view 
        return new DataClass(data1, data2, data3);
     }
    
  3. Now update your recyclerView inside onPostExecute method that gets called after doInBackground finishes executing. I'm convinced that it is not the best practice but I had to re-initialise adapter every time I wanted to update as I needed to pass the DataClass object to it.

    protected void onPostExecute(DataClass dataObject) {
     mRecyclerViewAdapter = new RecyclerViewAdapter(dataObject, .....);
     mRecylerView.setAdapter(mRecyclerViewAdapter);
     //Notify Data set or Item changed if required
    }
    
  4. DO NOT forget to write custom constructor for your recyclerViewAdapter in order to pass dataObject to it.

I solved my issue like this. The scroll is smooth except for the first time when the fragment is created.

Upvotes: 0

R. Zag&#243;rski
R. Zag&#243;rski

Reputation: 20268

The problem is, that you invoke this code inside onBindViewHolder. That means, that this code is executed for each newly shown row inside RecyclerView (and executed many times for the same row as you scroll!!).

How about moving this code to the Activity or Fragment (whatever is holding the list)? This way, the code will be executed once. Then wrap the data that you parse here into custom POJO object:

public class POJO {
    String activityValue;
    String activityValueText;
    String activityEmptyText;
    boolean activityCardEmptyViewVisibility;
    boolean activityCardViewVisibility;
}

Make such class the object you pass to RecyclerView. Then, inside onBindViewHolder use getItem(position) and just pass the data from POJO to Holder.

Do all the logic inside Activity (or any controller, pressenter, whatever is used for business logic). That is what it is destined for. Adapter is just for showing the results and not for creating business logic for your application.

Upvotes: 2

Related Questions