Passing Object as Params in AsyncTask

Issue

I wanted to query database using AsyncTask. Now I have option to make different AsyncTasks for each type of Query. Another option, that I am inclined to, is to make single AsyncTask for each type of query.

If I am passing params as objects and then casting them back, is this a bad practice? Can I run into some trouble? Should I be passing everything in constructor?

public void insert (Item item){
    new queryAsyncTask(mItemDao).execute(INSERT_QUERY, item);
}

public void delete (int uid){
    new queryAsyncTask(mItemDao).execute(DELETE_QUERY, uid);
}

private static class queryAsyncTask extends AsyncTask<Object, Void, Void>{

    private ItemDao mAsyncDao;

    queryAsyncTask(ItemDao dao){
        mAsyncDao = dao;
    }

    @Override
    protected Void doInBackground(Object... objects) {
        switch ((int) objects[0]){
            case DELETE_QUERY:
                mAsyncDao.deleteItem((int)objects[1]);
                break;
            case INSERT_QUERY:
                mAsyncDao.insert((Item)objects[1]);
                break;
        }
        return null;
    }
}

Solution

If I am passing params as objects and then casting them back, is this a bad practice? Can I run into some trouble? Should I be passing everything in constructor?

Yes, it is a very bad practice. Consider the following code of yours:

private static class queryAsyncTask extends AsyncTask<Object, Void, Void>{

    private ItemDao mAsyncDao;

    queryAsyncTask(ItemDao dao){
        mAsyncDao = dao;
    }

    @Override
    protected Void doInBackground(Object... objects) {
        ...
    }
}

You then can call it with:

new queryAsyncTask(mItemDao).execute(DELETE_QUERY, uid);

or

new queryAsyncTask(mItemDao).execute(INSERT_QUERY, item);

but then, you can also call it with both of the following:

new queryAsyncTask(mItemDao).execute(new Object(), item);
new queryAsyncTask(mItemDao).execute(new ArrayList<String>(), item);

which is didn't give any error whatsoever. It is because your code didn't have a strict limitation and didn't give enough explanation about what it does.

---

You're better to make each single Task for the CRUD and pass the value (reference) via the constructor. For example, you can create something like this for INSERT:

private static class InsertQueryTask extends AsyncTask<Void, Void, Void> {

    private ItemDao mItemDao;
    private Item mItem;

    InsertQueryTask(ItemDao dao, Item item) {
        mItemDao = dao;
        mItem = item;
    }

    @Override
    protected Void doInBackground(Void... voids) {
       mItemDao.insert(mItem);
    }
}

then you can call it:

new InsertQueryTask(mItemDao, item).execute();

The above code line is more readable and more maintainable because you can tell what the code is doing only by reading its name.

You can further modify your code to make it a Fluent Interface. Something like this:

private static class InsertQueryTask extends AsyncTask<Void, Void, Void> {

    private ItemDao mItemDao;
    private Item mItem;

    InsertQueryTask(ItemDao dao) {
        mItemDao = dao;
    }

    InsertQueryTask with(Item item) {
      mItem = item;
      return this;
    }

    @Override
    protected Void doInBackground(Void... voids) {
       mItemDao.insert(mItem);
    }
}

Now, you can call it with:

new InsertQueryTask(mItemDao).with(item).execute();

which is more readable than the previous code.

NOTE: All the code haven't tested yet.

Answered By - ישו אוהב אותך