Android: is onDestroy the new onStop?

Conventional Android development logic dictates that if there is some action you want to perform (or rather, stop performing) when your Activity is no longer visible to the user, do it in onStop(). Likewise, if there is some action you want to restart performing when the user restarts interacting with your Activity, do it in onStart(). The disadvantage of this approach, of course, is that it wouldn't play well with device orientation changes.

This post explores a couple of solutions to this problem, and concludes that there are cases where one has no choice but to postpone the actions that would be ideally taken in onStop(), to onDestroy().

A trivial (incorrect) example

public TrivialIncorrectActivity extends Activity{

    //onCreate() and other life-cycle overrides like onResume() go here ...

    @Override public void onStart(){
        super.onStart();
        startMakingThatPeriodicRestCall();
    }

    @Override public void onStop(){
        super.onStop();
        stopMakingThatPeriodicRestCall();
    }

    // ... Other life-cycle overrides like onDestroy() go here

}

This example is incorrect. Every time the user rotates the device, your app would stop making a REST call and then again start making the call. Not good at all.

setRetainInstance to the rescue . . .

API 11 introduced the Fragment API, and along with it, the setRetainInstance method, which is also usable with older versions of Android by means of the support library. You can go through the documentation to understand the effect of a setRetainInstance(true). Essentially, when a configuration change is happening, even though the hosting Activity is being re-created, the Fragment instance is not destroyed.

So, this allows us to improve upon our previous example.

public IncorrectRotationTolerantActivity extends FragmentActivity{

    private static final String TAG_RETAIN_FRAGMENT = "RetainFragment";

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        if(savedInstanceState == null){
            getSupportFragmentManager().beginTransaction()
                .add(IncorrectRetainFragment.newInstance(), TAG_RETAIN_FRAGMENT).commit();
        }
    }
}

public class IncorrectRetainFragment extends Fragment{

    public IncorrectRetainFragment(){}

    public static IncorrectRetainFragment newInstance(){
        IncorrectRetainFragment frag = new IncorrectRetainFragment();
        frag.setRetainInstance(true);
        return frag;
    }

    @Override
    public void onStart() {
        super.onStart();
        startMakingThatPeriodicRestCall();
    }

    @Override
    public void onStop() {
        super.onStop();
        stopMakingThatPeriodicRestCall();
    }

}

This code snippet still doesn't do what we want it to do. It does not prevent re-making that REST call during orientation changes. Why?

Because, setRetainInstance doesn't prevent a Fragment's onStop() from being called - it just prevents onDestroy() from being called. So, even if you ask for a Fragment instance to be retained across configuration changes, the onStop() method of the Fragment is always still called when the device is rotated.

onDestroy() is the new onStop()

To fix the problem, postpone stopping the REST call to the onDestroy() of the Fragment. Similarly, start making the call in onCreate() instead of in onStart(), since onCreate() is not called when the device is rotated, but onStart() is.

public RotationTolerantActivity extends FragmentActivity{

    private static final String TAG_RETAIN_FRAGMENT = "RetainFragment";

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        if(savedInstanceState == null){
            getSupportFragmentManager().beginTransaction()
                .add(RetainFragment.newInstance(), TAG_RETAIN_FRAGMENT).commit();
        }
    }
}

public class RetainFragment extends Fragment{

    public RetainFragment(){}

    public static RetainFragment newInstance(){
        RetainFragment frag = new RetainFragment();
        frag.setRetainInstance(true);
        return frag;
    }

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onStart(savedInstanceState);
        startMakingThatPeriodicRestCall();
    }

    @Override
    public void onDestroy() {
        super.onStop();
        stopMakingThatPeriodicRestCall();
    }

}

This seems so semantically wrong though. onDestroy() represents the end of the entire lifetime of an Activity/Fragment and what we really wanted to do was monitor the visible lifetime. Also, there is no guarantee that onDestroy() will ever be called. If you really try out this example on a phone or emulator, chances are that you'll never see the Rest call being stopped - at least not right away.

A more correct, more restrictive solution:

There exists another solution to this problem - but it works only on API 11 and later, because it uses methods introduced in API 11 - isChangingConfigurations() and getChangingConfigurations().

public RotationTolerantActivity extends FragmentActivity{

    private boolean mRotated;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Boolean nonConfigState =
            (Boolean)getLastCustomNonConfigurationInstance();
        if (nonConfigState == null) {
            mRotated = false;
        } else {
            mRotated = nonConfigState.booleanValue();
        }
    }

    @Override 
    public void onStart(){
        super.onStart();
        if(!mRotated){
            startMakingThatPeriodicRestCall();
        }
    }

    @Override
    public void onStop(){
        super.onStop();
        mRotated = false;
        if (isChangingConfigurations()) {
            int changingConfig = getChangingConfigurations();
            if ((changingConfig & ActivityInfo.CONFIG_ORIENTATION) == ActivityInfo.CONFIG_ORIENTATION) {
                mRotated = true;
            }
        }

        if(!mRotated){
               stopMakingThatPeriodicRestCall();
        }
    }

    @Override
    public Object onRetainCustomNonConfigurationInstance() {
            return mRotated ? Boolean.TRUE : Boolean.FALSE;
        }

}

This solution is semantically correct, and works as expected. However, it only works on API 11 and higher, even though we extend FragmentActivity from the support library .

Bonus: Why onStop() and not onPause()?

The keen reader would have observed that this post talks about stopping un-needed tasks in onStop()and not onPause() - even though onPause() is the only one of these methods that is guaranteed to be called. Remember that after onPause() is called, the process could be killed in order to reclaim memory and thus onStop() and onDestroy() might never be called.

Yet, this entire post insists on using onStop() to stop un-needed tasks. The reason for this lies in the technique used in my library android-app-pause. Unfortunately, this library in its current form does not handle device orientation changes correctly. This will be fixed in a future release though.

Code Smells: Calling Life-Cycle Methods of Program Components Explicitly

Here is some code I see that sets off alarm bells clanging in my head:

Explicitly calling life-cycle methods of components that are outside your control.

Since that sounds oh-so-generic, let me illustrate by way of example.

Assume you are writing a Servlet and your implementation doesn't care whether the request was a POST or a GET (in the current RESTful world, this should never be the case, but let's keep that aside for the purposes of this post). So, the following would appear to be a reasonable implementation:

public class MyServlet extends HttpServlet {
    protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
        //Handle the request 
    }

    protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
        doGet(req, resp); //Just call doGet
    }
}

I would never do this though. This is because doGet, doPost and the other doXXX methods are life-cycle methods of the Servlet. I have no control over when and how the servlet engine calls them. I also have no control over what the engine does once these methods return. It is possible that the engine performs some house-keeping tasks once a doGet returns, and that action could be different from what is done once a doPost returns.

It is really a simple matter of extracting the "common functionality" between such life cycle methods into a method and calling that method.

public class MyServlet extends HttpServlet {
    protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
        handleRequest(req, resp);
    }

    protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
        handleRequest(req, resp);
    }

    /**
     * Method that extracts out the common functionality between GET and POST requests
     */
    private void handleRequest(HttpServletRequest req, HttpServletResponse resp){
        //Handle the request
    }
}

What if the life cycle methods are in an Interface?

One opinion that came up in a discussion was that it is safe to call other life-cycle methods if they are in an interface as opposed to a class. Note that the above code sample extends javax.servlet.http.HttpServlet which is an abstract class as opposed to the base javax.servlet.Servlet which is an interface. This has come up specially while discussing callbacks.

I disagree with this opinion since it assumes that any book-keeping has to be performed by the same (possibly abstract) class that denotes the life-cycle component. This is not true. Take an imaginary UI toolkit for example, which has a Pane as a UI element. Now, assume an interface for handling interaction callbacks:

interface InteractionListener{
    public void onClick();
    public void onRightClick();
    public void onLongClick();
    public void onDoubleClick();
}

In this case again, assume that you want the same action to be performed on long-click and right-click. So, you might be tempted to do this:

Pane pane = // ...

pane.setInteractionListener(new InteractionListener(){
        public void onClick(){
            //Handle single-click
        }

        public void onRightClick(){
            //Handle right-click
        }

        public void onLongClick(){
            onRightClick();
        }

        public void onDoubleClick(){
            //Handle double-click
        }
});

This is still wrong. Yes- InteractionListener is an interface and there is no way it can have a concrete method that might perform house-keeping tasks. However, you have no control over how the UI toolkit engine invokes the call-backs. You also have no control over what action the engine takes after your onXXXClick() methods return. The solution, again is exactly the same as before: extract the common code into a method which you then invoke from both onLongClick() and onRightClick().

Conclusion

Avoid calling one component life-cycle method from another, when the component in question is outside your control. This could lead to unpredictable behavior. Simply refactor the common behavior into a method and call that method from both life-cycle call-backs.

A few other instances where I have seen this sort of code:

• Android's SqliteOpenHelper. In the onUpgrade() method, you probably want to take a backup of data, drop the tables, add new columns, and then re-create the tables. The implementation often looks like this:

  public class MySqliteOpenHelper extends SqliteOpenHelper{
      public void onCreate(SQLiteDatabase db){
          //Use SQL CREATE TABLE statements to create the tables.
      }
  
      public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion){
          /*
           * 1. Take backup of data
           * 2. Drop tables
           * 3. Add columns
           *
           * And finally:
           */
          onCreate(db); //This is not right.
      }
  }
  

  The right way to do this would be:

  public class CorrectSqliteOpenHelper extends SqliteOpenHelper{
      public void onCreate(SQLiteDatabase db){
          createDatabase(db);
      }
  
      public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion){
          /*
           * 1. Take backup of data
           * 2. Drop tables
           * 3. Add columns
           *
           * And finally:
           */
          createDatabase(db);
      }
  
      private void createDatabase(SQLiteDatabase db){
           //Use SQL CREATE TABLE statements to create the tables.
      }
  }
  

I will re-visit this post and add more instances as and when I come across them.