AsyncTask for image fetching keeps getting called

Issue

I have a small app for keeping list of students.

When adding a new student record, a random image url (string) is chosen from my own pool of 10 addresses, fetched & being set on an image view using AsyncTask.

I'm using custom CursorAdapter for the list, and custom SQLiteOpenHelper DB to handle the DB (contains id,name, grade, image url str).

I'm using a AsyncTask in order to fetch image from the internet

My problem is that my AsyncTask keeps getting called over and over again, upon every click on the screen, fetching the same image already fetched before.

I guess i'm using my AsyncTask incorrectly (Through bindView), but not sure.

My Goal is to fetch the image for every line only once

MainActivity:

public class MainActivity extends AppCompatActivity implements View.OnClickListener {

    /* Fields for adding new student to the list */
    private EditText mEtName;
    private EditText mEtGrade;
    private ListView mLvStudents;

    /* Our DB model to store student objects */
    private SqlDbHelper mDB;

    /* Custom SQL-Adapter to connect our SQL DB to the ListView */
    private SQLAdapter mAdapter;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        /* Init fields & needed views */
        mEtName = findViewById(R.id.et_name);
        mEtGrade = findViewById(R.id.et_grade);
        mLvStudents = findViewById(R.id.lv_students);
        mDB = new SqlDbHelper(getApplicationContext());
        mAdapter = new SQLAdapter(this, mDB.getAllRows(), false);

        /* Set click listeners and adapter to our list */
        mLvStudents.setAdapter(mAdapter);
        findViewById(R.id.button_add).setOnClickListener(this);
    }

    @Override
    public void onClick(View view) {
        final String name = mEtName.getText().toString();
        final int gradeInt = AidUtils.getGradeInt(mEtGrade.getText().toString());

        mDB.addStudent(name, gradeInt, AidUtils.randImageUrl());
        mAdapter.changeCursor(mDB.getAllRows());
        mEtName.setText("");
        mEtGrade.setText("");
    }
}

SQLAdapter:

final class SQLAdapter extends CursorAdapter {
    private LayoutInflater mInflater;

    public SQLAdapter(Activity context, Cursor c, boolean autoRequery) {
        super(context, c, autoRequery);
        mInflater = LayoutInflater.from(context);
    }

    @Override
    public View newView(Context context, Cursor cursor, ViewGroup viewGroup) {
        return mInflater.inflate(R.layout.lv_line, viewGroup, false);
    }

    @Override
    public void bindView(final View view, Context context, Cursor cursor) {
        /* Set name */
        ((TextView)view.findViewById(R.id.tv_name)).setText(
                cursor.getString(cursor.getColumnIndex(SqlDbHelper.KEY_NAME)));

        /* Set the image URL for it and fetch the image */
        final String imageUrlStr = cursor.getString(cursor.getColumnIndex(SqlDbHelper.KEY_IMG));
        ((TextView)view.findViewById(R.id.tv_image_url)).setText(imageUrlStr);

        new AsyncImageSet(imageUrlStr, (ImageView)view.findViewById(R.id.iv_pic)).execute();

        /* Set grade and color for it */
        final int grade = cursor.getInt(cursor.getColumnIndex(SqlDbHelper.KEY_GRADE));
        ((TextView)view.findViewById(R.id.tv_grade)).setText(String.valueOf(grade));
    }
}

SqlDbHelper:

final class SqlDbHelper extends SQLiteOpenHelper {
    private static final String TAG = "SqlDbHelper";

    /* Database version */
    public static final int VERSION = 1;

    /* Relevant string names, keys represent columns */
    public static final String DB_NAME = "StudentsDB";
    public static final String TABLE_NAME = "students";
    public static final String KEY_ID = "_id";
    public static final String KEY_NAME = "Name";
    public static final String KEY_GRADE = "Grade";
    public static final String KEY_IMG = "Image";

    public SqlDbHelper(Context context) {
        super(context, DB_NAME, null, VERSION);
    }

    @Override
    public void onCreate(SQLiteDatabase sqLiteDatabase) {
        StringBuilder createQuery = new StringBuilder();
        createQuery.append("CREATE TABLE " + TABLE_NAME + " (")
                .append(KEY_ID + " INTEGER PRIMARY KEY AUTOINCREMENT,")
                .append(KEY_NAME + " TEXT,")
                .append(KEY_GRADE + " INT,")
                .append(KEY_IMG + " TEXT")
                .append(")");

        Log.d(TAG, "Create table query: " + createQuery.toString());
        sqLiteDatabase.execSQL(createQuery.toString());
    }

    @Override
    public void onUpgrade(SQLiteDatabase sqLiteDatabase, int i, int i1) {}

    public void addStudent(final String name, final int grade, final String imageUrl) {
        ContentValues cv = new ContentValues();
        cv.put(KEY_NAME, name);
        cv.put(KEY_GRADE, grade);
        cv.put(KEY_IMG, imageUrl);
        getWritableDatabase().insert(TABLE_NAME, null, cv);
    }

    public Cursor getAllRows() {
        return (getReadableDatabase().
                rawQuery("SELECT * FROM " + TABLE_NAME, null));
    }
}

AsyncImageSet:

public class AsyncImageSet extends AsyncTask<Void, Void, Bitmap> {
    private String mImageUrl;
    private ImageView mImageView;

    public AsyncImageSet(String imageUrl, ImageView imageView) {
        mImageUrl = imageUrl;
        mImageView = imageView;
    }

    @Override
    protected Bitmap doInBackground(Void... voids) {
        Log.v("AsyncImageSet", "New Async Task launched!");
        Bitmap image = null;
        try {
            image = AidUtils.getBitmapFromUrl(AidUtils.buildUrl(mImageUrl));
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            return image;
        }
    }

    @Override
    protected void onPostExecute(Bitmap image) {
        if(image != null) {
            mImageView.setImageBitmap(image);
        }
    }
}

What Am I doing wrong here?

Thanks

Solution

What Am I doing wrong here?

You can't simply create a new AsyncTask and execute it in bindView(). That method is called every time a new row of your ListView enters the screen(and can be called in other situations too), so as the user scrolls your list up and down you'll create a lot of AsyncTask instances.

The proper way to handle this is to execute an AsyncTask to fetch an image only if there isn't an AsyncTask running already for that imageurl which you're trying to get. The simplest way to handle this is having a Map in your adapter to map a String(the imageUrl) to an AsyncTask instance(which will fetch the image pointed by that imageUrl):

final class SQLAdapter extends CursorAdapter {
    private LayoutInflater mInflater;
    private Map<String, AsyncImageSet> mappings = new HashMap<>();
    //...

and then, in your bindView() method us the map above:

//...   
final String imageUrlStr = cursor.getString(cursor.getColumnIndex(SqlDbHelper.KEY_IMG));
// at this point look in our map to see if we didn't already create an AsyncTask for this imageUrl
if (mappings.get(imageUrl) != null) {
// there's a task for this imageUrl already created so we use that
AsyncImageSet task = mappings.get(imageUrl);
task.updateView((ImageView)view.findViewById(R.id.iv_pic));
} else {
// there isn't a task for this imageUrl so create one and execute it(and save it in our mappings)    
AsyncImageSet task = AsyncImageSet(imageUrlStr, (ImageView)view.findViewById(R.id.iv_pic));  
mappings.put(imageUrl, task);
task.execute();
}
((TextView)view.findViewById(R.id.tv_image_url)).setText(imageUrlStr);
//...

You'll also need to change your AsyncTask to add the extra method:

public class AsyncImageSet extends AsyncTask<Void, Void, Bitmap> {
    //...
    private Bitmap bitmap; 

    public void updateView(ImageView imageView) {
         mImageView = imageView;
         // if the task is already finished it means the bitmap is 
         // already available
         if (getStatus() == AsyncTask.Status.FINISHED) {
              mImageView.setImageBitmap(bitmpa);
         }
    }

    @Override
    protected void onPostExecute(Bitmap image) {
    if(image != null) {
        bitmap = image;
        mImageView.setImageBitmap(image);
    }
}

This is a very simple implementation which will keep the bitmaps in memory which may not work if the images are big.

Ideally, as the other answer mentions, you should probably use a image loading library like Picasso which will help you avoid a lot of the pitfalls of implementing your own caching system.

Answered By - user