views:

55

answers:

1

In my Android app, when a user tries to transition from one activity to another, there may be some global state that indicates they need to complete some other action first.

To accomplish this, I've written a class with the following code:

private static WeakReference<Activity> oldActivityReference;
private static Intent waitingIntent;

public static void pushActivity(Activity currentActivity, Intent newActivityIntent) {
    Intent blockingIntent = ThisClass.getBlockingActivity();
    if (blockingIntent != null) {
        ThisClass.oldActivityReference = new WeakReference<Activity>(currentActivity);
        ThisClass.waitingIntent = newActivityIntent;
        currentActivity.startActivity(blockingIntent);
        return;
    }
    currentActivity.startActivity(newActivityIntent);
}

When the blocking activity finishes, it calls ThisClass.blockingActivityFinished(). That will check to see if the weak reference to the old activity still exists and, if so, launch the original intent from that activity. If not, it will launch the original intent from my application's context.

My question is,
Does this sound sane? Are there any potential memory leak issues with this technique? Is there a better way to accomplish this?

EDIT - To be clear, the types of events that might trigger an interruption are 1) a server ping indicating that the current app version is deprecated 2) any server RPC indicating that the user's credentials are no longer valid. I do not want to add logic to every Activity to handle checking for these, and resuming business as usual once they complete. That is a violation of DRY, and error-prone in a team environment.

+1  A: 

Does this sound sane?

I'd never use this technique. Mutable static data members are dangerous, WeakReference notwithstanding. In particular, I'd expect this to fail if the user does the unthinkable and, say, uses their phone as a phone, or otherwise leaves your application flow for an extended period of time. Your activities may be destroyed and your process terminated to free up RAM, yet the activities would remain in the task and might be reactivated. At that point, your state is whack, because the statics got nuked.

Are there any potential memory leak issues with this technique?

You're leaking an Intent.

Is there a better way to accomplish this?

For the purposes of the rest of this answer, I'm going to refer to your starting point as Activity A, the "some other action" as Activity B, and the desired end as Activity C. So, in your code, newActivityIntent is for Activity C, blockingIntent is for Activity B, and currentActivity is Activity A.

Option #1: Put the decision-making process in Activity C, rather than Activity A. Have Activity C check the condition in onCreate() and immediately calls startActivity() for Activity B if the conditions require Activity B to be shown.

Option #2: Leave the decision-making process in Activity A, but pass the boolean (e.g., true for "we gotta show Activity B") in an Intent extra for the startActivity() call for Activity C. Activity C checks the boolean in onCreate() and immediately calls startActivity() for Activity B if the boolean says so.

In these options, you avoid the statics.

CommonsWare
1) If the OS is down to destroying my activities, I'm not sure how this is unsafe? I mean, I do check if the Activity still exists. 2) Am I? I don't think that Intent is leaked. The only time it would leak is if the blocking activity did not properly call back, and even then it would get nulled out as soon as a new activity got launched. Intentional extended lifecycle != leak. 3) That is exactly what I'm trying to avoid. These are *global* states--eg the user loses authentication. Option 1 or 2 leads to a gross violation of DRY, and in a team environment is begging for bugs.
DougW
I'm not sure I understand that first part... what about this would cause Activities to remain in the task? Does a WeakReference somehow prevent an Activity from being released?
DougW
If instead of doing this statically, I made it a normal class and put an instance of it in my Application, would that assuage your concerns about the static-ness?
DougW
@DougW: " Option 1 or 2 leads to a gross violation of DRY, and in a team environment is begging for bugs." -- no, it doesn't. And even if it did, static data to avoid DRY is the wrong answer. "what about this would cause Activities to remain in the task?" -- this is independent of your technique. "If instead of doing this statically, I made it a normal class and put an instance of it in my Application, would that assuage your concerns about the static-ness?" -- not especially.
CommonsWare
@CommonsWare I think I get what you're saying, but I still think this fails-safe. If the statics get nuked, it's fine. The next transition the user attempts will just throw the block again. Also, how would your solutions support the case where I want to start C and finish immediately? I would have to keep A alive (not finish) until B is done, because A is the only thing that knows what C is. My solution basically just acts as a register for that information, so keeping A alive is not necessary.
DougW
@DougW: "I would have to keep A alive (not finish) until B is done, because A is the only thing that knows what C is." -- no. In both options, A starts C. C starts B if B is needed. C knows who C is, because C is C.
CommonsWare
@CommmonsWare Ah, yeah I misread that, makes sense. I think option #1 will probably work. Less fun than my method though ;) Thanks
DougW