Alternative to direct onResume() call

Question

I am rewriting my Android app to eliminate direct calls to onResume().

My app currently does most of its work inside onResume() it then posts the display and that is the end of onResume().

@Override
public void onResume() {
    super.onResume();
    // get current date and time,
    //  and determine if daylight savings time is in effect.
    //...600 lines of code
    // output both the chart and report
    //
    image.setImageBitmap(heightChart);
    report.setImageBitmap(reportBitmap);
}

The next step is gathering user input, which tells me what changes to the report the user wishes. (It may be a new location, a new date, or new display style, etc). This is done as follows:

@Override
public boolean onOptionsItemSelected(MenuItem item) {
    final int STATION_REQUEST = 1;
    int test = 1;
    switch (item.getItemId()) {
        case R.id.refresh: {
            userDateSet = false;
            onResume();
            return true;
        } // come to the present.

        //...200 lines of code
        default:
            return super.onOptionsItemSelected(item);
    }
}

As the example shows, the output is regenerated by calling onResume() after the new user command is determined. THIS IS BAD PRACTICE, I ALREADY KNOW!! Yet it works well as far as I have determined, I honestly do not understand the problem with it.

My solution in mind is to gather the 600 lines of code into a separate routine and call that instead, both from within onResume() and numerous points within onOptionsItemSelected()

@Override
public void onResume() {
    super.onResume();
    myOnResumeCode();
}

And inside onOptionsItemSelected() do this

@Override
public boolean onOptionsItemSelected(MenuItem item) {
    final int STATION_REQUEST = 1;
    int test = 1;
    switch (item.getItemId()) {
        case R.id.refresh: {
            userDateSet = false;
            myOnResumeCode();
            return true;
        } // come to the present.

    ... // Other statements
}

Is this method acceptable? If not, any suggestions short of "rewrite the whole thing" will be very helpful to me. I have searched extensively for a clean solution but not found one I can understand. Thank You.

Shellishellie

I am posting a bounty, because I would like a very clear statement about if my proposed solution is acceptable, if so why, great, if not why not and how else can I fix it. – Shellishellie

Answer 1

I honestly do not understand the problem with it.

Your onResume() method implementation is harmless by itself. But the call to it's super method super.onResume(); will let the system think that it is another occurrence of the resume event. It will lead to unnecessary resource usage for refreshing views and similar internal works. So you must avoid explicit calls to life-cycle callback methods under any circumstances.

Is this method acceptable?

The number of lines of code is doesn't make it acceptable or not. It is a question you need to ask yourself. If you think the whole code is to be executed at that event, then you should do it. Otherwise you could save some resources.

If you are doing something like this

public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
        case R.id.mnuEnableSomething:
            {
                refreshTheWholeUi();
                return true;
            }
        case R.id.mnuClearList:
            {
                refreshTheWholeUi();
                return true;
            }
    }
}

public void onResume() {
    super.onResume();
    refreshTheWholeUi();
}

Then changing it to this will be worth of it.

public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
        case R.id.mnuEnableSomething:
            {
                enableIt();
                return true;
            }
        case R.id.mnuClearList:
            {
                justClearTheList();
                return true;
            }
    }
}

public void onResume() {
    super.onResume();
    refreshTheWholeUi();
}

Now, to the core of the topic

After your reply, I had a closer look at your question and this struck my eyes.

My plan is to move those 600 lines to a separate class file. That will keep them away from damage while I work on the command decoder in the activity source file

Not actually. But you are really close. Forget about every complexities like activity life-cycle, methods, classes so and so and just focus to the very basic level of execution of a computer program.

A program is always executed line by line. It doesn't make any difference how did you arrange the code. Proper structuring of the program into methods, classes etc are for the sake of programmer's convenience. To the system, its always a series of lines. Therefore while performing heavy duties, the UI may became unresponsive as it have to wait until its turn.

Then how it is possible to do work in parallel?

Multi-Threading...!

It isn't that complicated as it sounds like.

You have to locate the most crucial part of your code which uses the resources more and move it to a different thread.

I have illustrated how to do multi-threading here.

public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
        case R.id.mnuProceessImageAction:
            {
                //Let user know that a background operation is running
                //with a progressbar or something
                processImage(mImage);
                return true;
            }
    }
}

private void processImage(Object image) {
    new Thread(new Runnable(){
        public void run() {
        //Doing all the heavy duty here.
        //.............................
        //Now you have the result. Use it to update the UI.
        //(UI can be updated only from the UI thread)
        runOnUiThread(new Runnable(){
                public void run() {
                    updateTheUiWithTheImage(proccessedImage);
                }
        });
        }
    }).start();

}

private void updateTheUiWithTheImage(Object image) {
    try {
        //Hide progressbar if you have one
        //Now it wont make the UI to struggle to use it.
    } catch(NullPointerException e) {
        e.printStackTrace;
    }
}

It is the most basic form. Of course there are alternatives(Like AsyncTask). You can find more about it easily online (Try searching "multi threading in Android"). Feel free to ask more.

Monkeypot

"But the call to it's super method super.onResume() will let the system think that it is another occurrence of the resume event." No, it won't. Once the system has called onResume() it "knows" the client (in this case Activity) is informed about the lifecycle event. It doesn't care what the client does further with its methods. – Harrar

Then why do you think that "The intention to use myOnResumeCode() without calling super.onResume() avoids the unnecessary method invocations and is a more optimized solution."? – Monkeypot

This is coming to the crux of how I see this problem. As I construct my solution, I can easily avoid the super call. Nobody has said it is dangerous NOT to call super. The suggestion to more carefully choose which commands need be accompanied by a complete recalculate is appealing. I need experiment to see if it can work in my case. I measured about 25 millisecond execution time for complete refresh on my Note 8. More like 50 milliseconds in power saving mode. – Shellishellie

I had a closer look on the question. I have updated the answer. I am sure it will help you. – Monkeypot

I needed do two things, moved the complexity of the main calculation physically out of the main source file, and clear up the direct onResume call issue. Most sustaining work on this app is in the command/response part, not the core calculation. Those two were in the same source file and it is very dangerous from editing errors. Now both things are done, it is working well. This is by nature a run and done app. (A daily tide prediction). It is about 1/8 second of data preparation, numeric calculation, and graphical output. – Shellishellie

I will try your proposed multi-threading method to move my heavy task to the background. Thank you, Ahamad. – Shellishellie

Answer 2

Yet it works well as far as I have determined, I honestly do not understand the problem with it.

You are assuming that calling super.onResume() is appropriate in cases where you are manually calling onResume(). That is not a safe assumption.

Is this method acceptable

It is certainly an improvement and well worth doing.

600 lines of code is a really long method. You would fail many code reviews, with reviewers asking you to refactor that code to be more maintainable. Also, depending on what you are doing in those 600 lines it may be better to move that logic to a background thread.

Bieber

My plan is to move those 600 lines to a separate class file. That will keep them away from damage while I work on the command decoder in the activity source file. I hope Java doesn't make that too excruciating. I would greatly prefer to have multiple compilation units for the activity source file but I cannot find a way for that. – Shellishellie

@user1644002: In modern Android app development, an activity does not do a whole lot directly. You override some lifecycle methods and other things that have to be in the activity, and you delegate pretty much everything else to something else (fragments, controllers/presenters, etc.). – Bieber

This on resume takes about 25 milliseconds on my note 8. I can hide the looping calculations in separate class files and it achieves my desire to clean up. There is one for tide calculations, one for sun times, and one for moon times and phase. Keeps it easier to control. But I don't hear a need to make major rewrite. I don't hear a fear of NOT calling super in my code.I probably won't hear an overriding black and white answer to my original question. But if it works it is good. I just don't like having to experiment to find out how it is supposed to work. – Shellishellie

@user1644002: "This on resume takes about 25 milliseconds on my note 8" -- that will drop 1-2 frames (16ms/frame, and there's other overhead in your UI beyond this onResume() call). You should consider moving this to a background thread. "I don't hear a fear of NOT calling super in my code" -- you are calling super.onResume() in your code, both in your original and your revised code. – Bieber

This is a run and done app. Unless the user wants a change it just sits there and displays the result. (It's a daily tide forecast) In my revised code I am calling myOnResumeCode(); and never onResume(); Sorry for any confusion thanks very much. – Shellishellie

Answer 3

Consider the Activity's onResume() source code which is executed every time super.onResume() is called:

protected void onResume() {
    if (DEBUG_LIFECYCLE) Slog.v(TAG, "onResume " + this);
    getApplication().dispatchActivityResumed(this);
    mActivityTransitionState.onResume();
    mCalled = true;
}

where mActivityTransitionState.onResume() invokes resetViews() with further actions on views in the window. So while you experienced that it works, all the method invocations are wasting the CPU time and, in fact, redundant, which brings to a conclusion the 1st approach is inefficient.

On the other hand, the intention to use myOnResumeCode() without calling super.onResume() avoids the unnecessary method invocations and is a more optimized solution.

Moreover, 600 lines of code is quite a big amount. If those lines run on the main thread it freezes UI making the app look less responsive. It is better to do the calculations on a background thread and post the view changes on the main thread.

I don't hear a fear of NOT calling super in my code.

It seems to be a misunderstanding of what Activity's lifecycle methods are meant for. Those methods are callbacks the system use to inform listeners about events occurring in it. And you got such a notification once onResume() was called by the system. If you remove super.onResume() you'll get an exception, it's clearly stated in the source code linked and is the only request the system "wants" the Ativity to perform when calling its onResume(). It applies to all the other lifecycle methods - to be informed by OS. The system does not care if Activity calls them again, "manually". While the Activity is in foreground, you can call onResume() as much as you want wasting CPU time and making your app look less responsive to users.

Again, overridden onResume() is a callback ("listener") and cannot effect the system behavior in contrast to, e.g. the finish() method that effect the system behavior saying: "Hey, system, I'm all done and wanna be killed by you". Those kind of methods may be treated as requests to the system.

Update

Do they even tell you where to put your application code?

You're free to put your code everywhere you want. The system just informs you through the lifecycle method invocations when, say, your content is visible to user or hidden. So it's a question of putting your code in a reasonable "place" in accordance to lifecycle events.

Does this state it is OK to directly call onResume()? There is such strongly stated prohibitions expressed against that.

It's pointless, but works as you witnessed. "You shall not eat meat on Friday", but who said you can't? :)

Harrar

Onik, I appreciate the commentary. Unfortunately all of my explanation, disguised the question: if there is indeed an issue calling onResume() directly, can I resolve that issue by separating out that into two parts, the super call followed by my own routine. And call my own routine to update the display when my command decoder finishes. Does that resolve the strongly-stated mandate not to call onResume() myself. Thanks and I will read your suggested items, at this time I am just trying to clear this issue. BTW the delay is imperceptible. – Shellishellie

if I dive into my own onResume code and just do the last 99% (the part after the super call) is it safe. the time this happens is when my user has given me a command by touch or swipe thru onOptionsItemSelected. – Shellishellie

Onik, I was discussing "myOnResume()" when I described not calling super. myOnResume is identical to onResume except for the super call being removed. The working code in myOnResume, including the final bitmap outputs, work fine without the super call preceeding it. – Shellishellie

I remember struggling with the lifecycle explanations when I wrote this app. Do they even tell you where to put your application code? – Shellishellie

Regarding this statement: The system does not care if Activity calls them again, "manually". Does this state it is OK to directly call onResume()? There is such strongly stated prohibitions expressed against that. Sort of like "You shall not eat meat on Friday". – Shellishellie

Well, in this case it is as easy to do it the recommended way, breaking up the onResume to two pieces, and it is not necessary to litigate the question to solve the problem. The proposed arrangement has been coded and it is working well. – Shellishellie

Answer 4

Yet it works well as far as I have determined, I honestly do not understand the problem with it.

I think @CommonsWare and others have already pointed out the problem you will be having in case of calling the onResume function while updating your UI elements again on the basis of user interaction.

Is this method acceptable? If not, any suggestions short of "rewrite the whole thing" will be very helpful to me.

600 lines of code is not maintainable always. You might consider breaking them up into several functions. However, from my point of view, it is still a difficult job to pull off all the things together from a single place.

I would highly recommend you to use ViewModel in your case. The implementation and management will become a lot simpler. I am attaching the sample implementation from the developer documentation.

public class UserActivity extends Activity {

     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         setContentView(R.layout.user_activity_layout);
         final UserModel viewModel = ViewModelProviders.of(this).get(UserModel.class);
         viewModel.userLiveData.observer(this, new Observer() {
            @Override
             public void onChanged(@Nullable User data) {
                 // update ui.
             }
         });
         findViewById(R.id.button).setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
                  viewModel.doAction();
             }
         });
     }
 }

ViewModel would be like this.

 public class UserModel extends ViewModel {
     public final LiveData<User> userLiveData = new LiveData<>();

     public UserModel() {
         // trigger user load.
     }

     void doAction() {
         // depending on the action, do necessary business logic calls and update the
         // userLiveData.
     }
 }

On your action on some data which resides in your ViewModel will get the UI updated as we have implemented the callback function which is onChange.

The idea of implementing callback functions can be done in several ways (e.g. defining an interface and then override the function) as well. It makes a code a lot cleaner if they can be implemented rightly.

And from documentation of ViewModel...

ViewModel can also be used as a communication layer between different Fragments of an Activity. Each Fragment can acquire the ViewModel using the same key via their Activity. This allows communication between Fragments in a de-coupled fashion such that they never need to talk to the other Fragment directly.

public class MyFragment extends Fragment {
     public void onStart() {
         UserModel userModel = ViewModelProviders.of(getActivity()).get(UserModel.class);
     }
}

Now I think, your problem gets much simpler. You might also consider breaking up your UI elements in several Fragment and handle the updates there with different life-cycle elements.

Hope that helps!

Dionnadionne

Reaz, My application is broken up into numerous classes and activities, totalling about 10 or 12. The act of creating output is now accomplished during onResume by calling a separate routine so no super call involved on my part. The system tells me when I can run by calling on Resume. My app tells me when I can rerun the same code from onOptionsItemSelected action. I appreciate the input and am working to use it effectively, thank you. – Shellishellie

Answer 5

Here is my final coding, thanks to Ahamad for the excellent guidance. I never call onResume() now. In various places, I call myOnResume(). The part1() code is all the extended calculations. The part2() code is all the output operations.

@Override
    public void onResume()
    {   super.onResume();//
        myOnResume();
    }// end: onResume.

    void myOnResume()
    {   new Thread(new Runnable()
        {   public void run()                   //run #1
            {   part1();                        // do all the calculations in background.
                runOnUiThread(new Runnable()
                {   public void run()           //run #2
                    {   part2();                // do the display in the UI thread.
                    }                           //end of run#2
                });                             //end of runOnUiThread
            }                                   //end of run#1
        }).start();
    }                                           //end: myOnResume()

Shellishellie

Answer 6

This is a very common habit in developers to call the life cycle methods to perform certain tasks because it is convenient.

But the actual code must be properly modularised.

Since you are rewriting the code, I would recommend you to migrate to MVVM or MVP architecture because managing the cases which you mentioned is better with these Architectures.

Whether you use and architecture or not, it is good to split the code based on purpose.

For example onResume() means what you do when Activity/Fragment resumes. And same applies for onCreate() , onDestroy() etc.

In general terms,
1. We initialise non changing variables in onCreate and release them in onDestroy.
2. We re fetch the data from backend or refresh the UI in onResume()
3. We pause any on going process like media playback in onPause()
etc

As per your code example you mentioned about 600 lines, which I assume does not perform same task.

So you must split the code based on task

private void refreshDayLightTime(){
    //refresh your time
}

private void performRefreshTasks(){
   //series of codes that are to be executed one after the other
   //ideally this method should call methods which perform specific tasks.
}

private void updateLaunchViews(Bitmap heightChart, Bitmap reportBitmap){
   image.setImageBitmap(heightChart);
   report.setImageBitmap(reportBitmap);
}

Using this approach keeps your code clean. And mainly you don't end up breaking the life cycles of the app. And you can control which action to be called from which part of the app.

Wellesley

Mohammed, My arrangement does organize as your 1,2,3. The change today is to remove my direct onResume call which has been done, and separate out as possible tasks to different pieces of source code (class and activity files). But there is no ongoing work in this application, it's a 1/8 second burst of activity only repeated if ever the user does a refresh or changes the location or style of display. If I were to add for example a yearly prediction, I agree a re-architected solution would be required. – Shellishellie

Answer 7

It is always advisable to use any approachable / maintainable code.

I would rather suggest to not only move your code to a maintainable piece , but also refactor the implementation by following any of methods which separate's the presentation layer from the logic.

For instance Android Model View Presenter / MVP with working example

Now if you could go through more simple explanation (Simpler explanation for MVP), definitely the possibility of easier debugging, unit testing and maintainable code is evident.

Coming to your points (which already @CommonsWare) has explained much, moving all the 600 lines of code to Background Thread or Async task will improve the performance of your app. Now you will not see anymore message like below.

I honestly do not understand the problem with it

---

Statement reference what could be included in onResume() from developer.android

Therefore, any method that runs in the UI thread should do as little work as possible on that thread. In particular, activities should do as little as possible to set up in key life-cycle methods such as onCreate() and onResume(). Potentially long running operations such as network or database operations, or computationally expensive calculations such as resizing bitmaps should be done in a worker thread (or in the case of databases operations, via an asynchronous request)

---

Android life methods are supposed to be invoked by system , by calling the super onResume / super onPause.This allows system to allocate/deallocate resources. Of course we can extend the usability by calling child methods inside onResume() , onPause() etc. But keeping business logic in these methods and calling those are not at all advisable.

In case of MVP, below are the guild lines that can be followed. As of now there is no standard/solid definition , how to achieve the same. But still the sample I have provided is a good place to start.

1. Separate code for View. Keeping Views as dumb as possible
2. Keep all business logic in Presenter classes
3. Models are interfaces responsible for managing data

Prepotent

Sreehari, I appreciate the work you have made to help me. Respectfully, I am an engineer and have been all the past 40 years, and I believe in specifications. Is there a document from the OS authors which limits the use of onResume for what you call business logic. I have heard much mention of so many lines of code, but that is not an indicator of resource usage obviously. I can state so many bytes of memory for so many milliseconds. That is a Spec based approach. What is the specification. I can perform in in a modular fashion that is frankly my problem to solve and maintain. Thanks. – Shellishellie

At a high level, my app creates the output once during a call from onResume. It next creates the output when a new command is available, from a call to the same routine from within onOptionsItemSelected. If I pass off that create output to a background routine, it still has to be activated from either of those two places. Honestly, to me this is a very common approach to a user program. It runs when started, and it runs when responded to. Otherwise it is just laying there in waiting, holding it's memory. – Shellishellie

Agreed that , there is no official documentation.What I have given out,is based out of minimal experience that I had.Let me brief out what could be done in onResume. Which may eventually explain what not to be done :) (1)Event is just an announcement by the OS that it has resumed the Activity and that we can start accessing app or hardware items such as the bluetooth, camera etc(2)Unpausing any thread which was running prior to moving out of Activity.(3)Initiated API request,response didn't received/completed.But still want to sync with other methods for completion. – Prepotent

(4)Part of program which cant bring back results , but still needs completion.Now comes AsyncTask and Threads. Good Luck :) – Prepotent

developer.android.com/training/articles/perf-anr Therefore, any method that runs in the UI thread should do as little work as possible on that thread. In particular, activities should do as little as possible to set up in key life-cycle methods such as onCreate() and onResume(). Potentially long running operations such as network or database operations, or computationally expensive calculations such as resizing bitmaps should be done in a worker thread (or in the case of databases operations, via an asynchronous request) @Shellishellie – Prepotent

There you go, there is proof for the strong prohibition, thanks again Sreehari. I am working with Ahamad's proposal (above) to see if I can get this done right. Larry – Shellishellie

Glad I could help :) – Prepotent