Android Development - Faulty implementation of AsyncTask in Fragment?

Question

I recently began trying to implement AsyncTasks in the Fragments of my Android Project, and immediately ran into the common error of a configuration change causing a major problem. The methods I saw online did not easily incorporate into the ViewPager setup I have, and I used my knowledge to incorporate handling the configuration change myself.

My question is: Is there anything dangerous on my approach? The biggest concern is a memory leak, but I made sure to null out each created_View on the onDetach() method.

Summary of Implementation:

• setRetainInstance to true for Fragment so it does not have to be recreated and important data is not lost.

• In onCreateView(), the section of code always called when a Fragment's View has to be recreated, the application will check to see if its AsyncTask is running. If so, display an IndeterminateProgressBar and when it finishes the onPostExecute, change the visibility of it to GONE.

• In onDetach(), make sure the created_view View is set to null so there is no memory leak related to the Activity that is initially used

• in the onAttach before the configuration change.

Code

public class RosterFragment extends Fragment 
{

List<RosterMember> dataforroster = new ArrayList<RosterMember>(); //List that will hold the Roster objects retrieved from Parse database,
                                 //and later passed in to constructor for the RosterCustomArrayAdapter.
List<ParseUser> retrieved_list = new ArrayList<ParseUser>(); //List that will hold values retrieved from ParseUser Query. 
View createdView; //View that will be passed back with built RosterFragment
private ProgressBar roster_progress; //The indeterminate ProgressBar that will be displayed until the AsyncTask is finished downloading the roster.
boolean running_task;
private RosterAsyncTask get_roster;

@Override
public void onCreate(Bundle savedInstanceState)
{
    super.onCreate(savedInstanceState);
    // Retain this fragment across configuration changes.
    setRetainInstance(true);
    get_roster = new RosterAsyncTask(); //Create new RosterAsyncTask instance.
    get_roster.execute();
    running_task = true;

}

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,Bundle savedInstanceState) 
{
    createdView = inflater.inflate(R.layout.rosterfragment, container, false); //Inflate the fragment using the specific layout
    roster_progress = (ProgressBar) createdView.findViewById(R.id.loadingroster); //Find the ProgressBar in layout and set it to roster_progress.
    if(running_task == true)
    {
        roster_progress.setVisibility(View.VISIBLE);
    }
    else
    {
        fill_roster();
    }
    return createdView;
}

@Override
public void onDetach()
{
    super.onDetach();
    createdView = null;
}


public void fill_roster()
{
    if(!dataforroster.isEmpty())
    {
    //Get reference ListView in the inflated layout.
    ListView the_Roster = (ListView) createdView.findViewById(R.id.rostercoachofficers);
    //Create an instance of the RosterCustomArrayAdapter using the dataforroster List.
    RosterCustomArrayAdapter roster_Adapter = new RosterCustomArrayAdapter(getActivity(), dataforroster);
    //Sort the roster_Adapter so elements in ListView will be sorted alphabetically by first name.
    roster_Adapter.sort(new RosterComparator());    
    //Attach adapter to the ListView to populate its data.
    the_Roster.setAdapter(roster_Adapter);
    }

}


//AsyncTask responsible for downloading roster in background thread.
private class RosterAsyncTask extends AsyncTask<Void, Void , List<RosterMember>>
{


    //The operations to perform in the AsyncTask background thread. The results(the roster data downloaded) will be passed to onPostExecute.
    @Override
    protected List<RosterMember> doInBackground(Void... params)
    {
        SystemClock.sleep(10000);
        ParseQuery<ParseUser> query = ParseUser.getQuery(); //Get specific ParseQuery for ParseUsers.

        try 
        {
        retrieved_list = query.find(); //Initiate query.
        for(ParseUser current_user: retrieved_list) //For every ParseUser returned from query, create a new RosterMember using the ParseUser
                               //data and then add it to the dataforroster List.
            {
                RosterMember current_member = new RosterMember();
                current_member.username = current_user.getUsername();
                ParseFile parse_ByteArray = (ParseFile)current_user.get("profile_picture");
                Bitmap profile_Picture = BitmapFactory.decodeByteArray(parse_ByteArray.getData(), 0, parse_ByteArray.getData().length);
                current_member.profile_Picture = profile_Picture;
                current_member.title = current_user.getString("title");
                dataforroster.add(current_member);
            }




        } 

        //If problem occurred in query execution, use Toast to display error message.
        catch (ParseException e) 
        {
            Toast.makeText(getActivity(), "Error, " + e.getMessage(), Toast.LENGTH_LONG).show();
        }
        return dataforroster;
   }



  //Code to run in main UI thread once the doinBackground method is finished.
  @Override
  protected void onPostExecute(List<RosterMember> dataforroster) 
  {    
      running_task = false; 
      fill_roster();
      roster_progress.setVisibility(View.GONE);

  }



}

}    

Answer 1

public class RosterFragment extends Fragment 
{
    List<RosterMember> dataforroster = new ArrayList<RosterMember>(); //List that will hold the Roster objects retrieved from Parse database,
                                     //and later passed in to constructor for the RosterCustomArrayAdapter.
    List<ParseUser> retrieved_list = new ArrayList<ParseUser>(); //List that will hold values retrieved from ParseUser Query. 
    View createdView; //View that will be passed back with built RosterFragment
    private ProgressBar roster_progress; //The indeterminate ProgressBar that will be displayed until the AsyncTask is finished downloading the roster.
    boolean running_task;
    private RosterAsyncTask get_roster;
    private boolean successful_query;
    private String error_message;

    @Override
    public void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        // Retain this fragment across configuration changes.
        setRetainInstance(true);
        get_roster = new RosterAsyncTask(); //Create new RosterAsyncTask instance.
        get_roster.execute();
        running_task = true;

    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container,Bundle savedInstanceState) 
    {
        createdView = inflater.inflate(R.layout.rosterfragment, container, false); //Inflate the fragment using the specific layout
        roster_progress = (ProgressBar) createdView.findViewById(R.id.loadingroster); //Find the ProgressBar in layout and set it to roster_progress.
        fill_roster();
        return createdView;
    }

    @Override
    public void onDetach()
    {
        super.onDetach();
        createdView = null;
    }


    public void fill_roster()
    {

            if(running_task == true)
            {
                roster_progress.setVisibility(View.VISIBLE);
            }
            else
            {

                roster_progress.setVisibility(View.GONE);

                if(!dataforroster.isEmpty())//Get reference ListView in the inflated layout.
                {
                    ListView the_Roster = (ListView) createdView.findViewById(R.id.rostercoachofficers);
                    //Create an instance of the RosterCustomArrayAdapter using the dataforroster List.
                    RosterCustomArrayAdapter roster_Adapter = new RosterCustomArrayAdapter(getActivity(), dataforroster);
                    //Sort the roster_Adapter so elements in ListView will be sorted alphabetically by first name.
                    roster_Adapter.sort(new RosterComparator());    
                    //Attach adapter to the ListView to populate its data.
                    the_Roster.setAdapter(roster_Adapter);

                }
                else
                {
                    if(successful_query == false)
                    {
                        Toast.makeText(getActivity(), error_message, Toast.LENGTH_LONG).show();
                    }
                }
            }


    }




    //AsyncTask responsible for downloading roster in background thread.
    private class RosterAsyncTask extends AsyncTask<Void, Void , Void>
    {


        //The operations to perform in the AsyncTask background thread. The results(the roster data downloaded) will be passed to onPostExecute.
        @Override
        protected Void doInBackground(Void... params)
        {
            dataforroster.clear();
            ParseQuery<ParseUser> query = ParseUser.getQuery(); //Get specific ParseQuery for ParseUsers.

            try 
            {
            retrieved_list = query.find(); //Initiate query.
            for(ParseUser current_user: retrieved_list) //For every ParseUser returned from query, create a new RosterMember using the ParseUser
                                   //data and then add it to the dataforroster List.
                {
                    RosterMember current_member = new RosterMember();
                    current_member.username = current_user.getUsername();
                    ParseFile parse_ByteArray = (ParseFile)current_user.get("profile_picture");
                    Bitmap profile_Picture = BitmapFactory.decodeByteArray(parse_ByteArray.getData(), 0, parse_ByteArray.getData().length);
                    current_member.profile_Picture = profile_Picture;
                    current_member.title = current_user.getString("title");
                    dataforroster.add(current_member);
                }


            successful_query = true;        

            } 

            //If problem occurred in query execution, use Toast to display error message.
            catch (ParseException e) 
            {
                successful_query = false;
                error_message = "Error, " + e.getMessage();
            }
            return null;

       }



      //Code to run in main UI thread once the doinBackground method is finished.
      @Override
      protected void onPostExecute(Void ignore) 
      {    

          running_task = false; 

          if(getActivity()!=null)
          {
              fill_roster();
          }

      }



    }   
}

Answer 2

The two main things I can see both deal with your activity. Your toast in doInBackground() needs to be called from your main thread, so I'm fairly sure that will fail. The activity you pass to the adapter is also separate from the fragment, so whenever you have a configuration change that will be destroyed and will no longer be valid.

To help prevent any force closes you may want to replace the toast with a message to the logcat and verify that the activity and roster_progress are not null before doing anything in onPostExecute.