1
\$\begingroup\$

I just read this post on how to avoid memory leaks with AsyncTask. The post proposed using a WeakReference and supplying a TextView as the object of the WeakReference.

In my code, I need to supply multiple parameters to the AsyncTask (not just one as shown in the post). So what I did was create an inner class with the parameters needed in the AsyncTask and use the said class as the object of the WeakReference.

private static class AddBooksToDatabase extends AsyncTask<String, String, String> {
        private final WeakReference<AddBooksDbParams> mReference;
        private String TAG = "TownFragment";
        Context mContext;
        WaveLoadingView waveView;
        TextView infoText;
        String townName;
        File mFile;

        public AddBooksToDatabase(AddBooksDbParams params) {
            this.mReference = new WeakReference<    >(params);
            mContext = mReference.get().mContext;
            infoText = mReference.get().infoText;
            townName = mReference.get().townName;
            mFile = mReference.get().mFile;
            waveView = mReference.get().waveView;
        }

        @Override
        protected String doInBackground(String... strings) {
            TownHelper helper = TownHelper.getInstance(mContext, dbName);
            SQLiteDatabase database = helper.getWritableDatabase();
            int booksSize = getFilesInFolder(mFile).size();
            //Stuffs
            return null;
        }

        @Override
        protected void onPreExecute() {
            if (waveView != null) {
                waveView.setVisibility(View.VISIBLE);
            }
        }

        @Override
        protected void onPostExecute(String s) {
            if (waveView != null) {
                waveView.setVisibility(View.GONE);
            }
        }

        @Override
        protected void onProgressUpdate(String... values) {
            super.onProgressUpdate(values);
            Log.d(TAG, "Progress report = " + values[0]);
            infoText.setText(values[0]);
        }

        @Override
        protected void onCancelled() {
            cancel(true);
        }
    }


    //Parameters for AddBooksToDatabase. This is to enable holding of a
    //single object of this class in WeakReference
    private class AddBooksDbParams {
        Context mContext;
        WaveLoadingView waveView;
        TextView infoText;
        String townName;
        File mFile;

        AddBooksDbParams(TextView infoText, Context context, File file,
                         String townName, WaveLoadingView waveView) {
            this.infoText = infoText;
            mContext = context;
            mFile = file;
            this.townName = townName;
            this.waveView = waveView;
        }
    }

When I want to execute the AsyncTask:

AddTownsDbParams params = new AddTownsDbParams(infoText, getActivity(), folder, mShelfLabel, mWave);
addBooksTask = new AddBooksToDatabase(params).execute();

The code is working quite aright but I want to know if I am doing wrong.

\$\endgroup\$
2
  • \$\begingroup\$ Is there a reason you're not just using multiple WeakReferences? i.e. WeakReference<Context>, WeakReference<WaveLoadingView>, WeakReference<TextView>, WeakReference<String>, WeakReference<File>? \$\endgroup\$
    – Moses
    Dec 14 '16 at 11:39
  • \$\begingroup\$ @Moses I never knew I could do it that way. But now that I know, I don't think I can use it because of entering multiple WeakReference<...> in the AsyncTask constructor except my code is not okay. \$\endgroup\$
    – X09
    Dec 14 '16 at 13:53
1
\$\begingroup\$

What about this

private static class AddBooksToDatabase extends AsyncTask<String, String, String> {

  private String TAG = "TownFragment";

  private WeakReference<Context> mContextRef;
  private WeakReference<TextView> mInfoTextRef;
  private WeakReference<String> mTownNameRef;
  private WeakReference<File> mFileRef;
  private WeakReference<WaveLoadingView> mWaveViewRef;

  public AddBooksToDatabase(Context context, TextView infoText, String townName, File file, WaveLoadingView waveView) {
    mContextRef = new WeakReference<>(context);
    mInfoTextRef = new WeakReference<>(infoText);
    mTownNameRef = new WeakReference<>(townName);
    mFileRef = new WeakReference<>(file);
    mWaveViewRef = new WeakReference<>(waveView);
  }

  @Override
  protected String doInBackground(String... strings) {
    TownHelper helper = TownHelper.getInstance(mContextRef, dbName);
    SQLiteDatabase database = helper.getWritableDatabase();
    int booksSize = getFilesInFolder(mFileRef).size();
    //Stuffs
    return null;
  }

  @Override
  protected void onPreExecute() {
    WaveLoadingView waveView = mWaveViewRef.get();
    if(waveView != null) {
      waveView.setVisibility(View.VISIBLE);
    }
  }

  @Override
  protected void onPostExecute(String s) {
    WaveLoadingView waveView = mWaveViewRef.get();
    if(waveView != null) {
      waveView.setVisibility(View.GONE);
    }
  }

  @Override
  protected void onProgressUpdate(String... values) {
    super.onProgressUpdate(values);
    Log.d(TAG, "Progress report = " + values[0]);
    TextView infoText = mInfoTextRef.get();
    infoText.setText(values[0]);
  }

  @Override
  protected void onCancelled() {
    cancel(true);
  }
}

Without seeing the surrounding code or understanding too much about it's purpose/function, here is how I would change the code

private static class AddBooksToDatabase extends AsyncTask<String, String, String> {

  private String TAG = "TownFragment";

  private WeakReference<Context> mContextRef;

  private String mTownName;
  private File mFile;

  private Dialog mProgressDialog;

  private TextView mInfoText;
  private WaveLoadingView mWaveView;

  /*
  * A constructor/method should rarely exceed 3 parameters.
  * */
  public AddBooksToDatabase(Context context, String townName, File booksFolder) throws IOException {
    /*
    * Since the file parameter must be a directory add in this
    * check as early as possible and throw a checked exception
    * which the caller must handle if it fails.
    * */
    if(!booksFolder.isDirectory()) {
      throw new IOException("the file " + booksFolder.getName() + " is not a directory");
    }
    /*
    * Holding a WeakReference to the Context because
    * the AsyncTask should not outlive the Context.
    * */
    mContextRef = new WeakReference<>(context);

    mTownName = townName;
    mFile = booksFolder;

    /*
    * The progress views are initialised inside the
    * AsyncTask, no need to pass them in as arguments.
    * */
    mInfoText = new TextView(context);
    mWaveView = new WaveLoadingView(context);

    /*
    * Create a dialog to display the progress views.
    * */
    LinearLayout progressDialogView = new LinearLayout(context);
    progressDialogView.setOrientation(LinearLayout.VERTICAL);
    progressDialogView.addView(mInfoText);
    progressDialogView.addView(mWaveView);
    mProgressDialog = new AlertDialog.Builder(context)
        .setTitle("Adding books")
        .setView(progressDialogView)
        .create();
  }

  @Override
  protected void onPreExecute() {
    /*
    * Display the progress dialog
    * when the task starts executing.
    * */
    mProgressDialog.show();
  }

  @Override
  protected String doInBackground(String... strings) {
    final Context context = mContextRef.get();
    TownHelper helper = TownHelper.getInstance(context, dbName);
    SQLiteDatabase database = helper.getWritableDatabase();
    int booksSize = getFilesInFolder(mFile).size();
    //Stuffs
    return null;
  }

  @Override
  protected void onProgressUpdate(String... values) {
    super.onProgressUpdate(values);
    Log.d(TAG, "Progress report = " + values[0]);
    mInfoText.setText(values[0]);
  }

  @Override
  protected void onPostExecute(String s) {
    /*
    * Dismiss the progress dialog
    * when the task finishes executing.
    * */
    mProgressDialog.dismiss();
  }

  @Override
  protected void onCancelled() {
    cancel(true);
  }

}
\$\endgroup\$
2
  • \$\begingroup\$ Thanks Moses for the answer but please can you help me nit-pick the code? ie tells what's wrong with it. \$\endgroup\$
    – X09
    Dec 14 '16 at 17:40
  • \$\begingroup\$ I have edited my answer with more suggestions. I will need more information to suggest any further changes/edits. \$\endgroup\$
    – Moses
    Dec 15 '16 at 11:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.