New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert AsyncTask to IntentService #10

Merged
merged 7 commits into from Jun 1, 2015

Conversation

Projects
None yet
2 participants
@yulin2
Contributor

yulin2 commented May 30, 2015

Hi UltimateAndroid developers, I'm doing research on Android async programming, particularly on
AsyncTask and IntentService. AsyncTask can lead to memory leak and losing task result when GUI is recreated during task running (such as orientation change). This article describes the problems very well.

We discussed with some Android experts and they agree with this issue, and claim that AsyncTask can be considered only for short tasks (less than 1 second). However, using IntentService (or AsyncTaskLoader) can avoid such problems since their lifecycles are independent from Activity/Fragment.

In UltimateAndroid, the AsyncTasks are declared as non-static inner classes of Activity/Fragment. So the above issue can occur (especially for relatively long tasks).

I refactored 6 AsyncTasks in UltimateAndroid to IntentService (in 6 separate commits), with the help of a refactoring tool we developed. Do you think using IntentService should be better in some of these 6 scenario? Do you want to merge some commits in this pr? Thanks.

@cymcsg

This comment has been minimized.

Show comment
Hide comment
@cymcsg

cymcsg May 31, 2015

Owner

Thank you very much for giving such useful PR!
I think it's a good idea to use IntentService instead of AsyncTasks in some cases.

In fact,the UltimateAndroidNormal has been deprecated so I think you do not need to pay too much time on that and I think there are some bug in ProgressMenuItemActivity in your commit a847e2d .However I haven't read it clearly only do some test on it .Could you help me find if there were some bugs on that which cause the app crashes.

Thank you very much and I want no only merge these PR but also find a way to propagandize that.

Owner

cymcsg commented May 31, 2015

Thank you very much for giving such useful PR!
I think it's a good idea to use IntentService instead of AsyncTasks in some cases.

In fact,the UltimateAndroidNormal has been deprecated so I think you do not need to pay too much time on that and I think there are some bug in ProgressMenuItemActivity in your commit a847e2d .However I haven't read it clearly only do some test on it .Could you help me find if there were some bugs on that which cause the app crashes.

Thank you very much and I want no only merge these PR but also find a way to propagandize that.

@yulin2

This comment has been minimized.

Show comment
Hide comment
@yulin2

yulin2 May 31, 2015

Contributor

Oh, I don't know UltimateAndroidNormal is deprecated, so I change it instead of UltimateAndroidGradle. I found the same files are also in UltimateAndroidGradle, so you can apply the same conversions for UltimateAndroidGradle.

For ProgressMenuItemActivity, the receiver should be put in onCreateOptionsMenu. I commit this change in 0cb7d85 and now it works.

Contributor

yulin2 commented May 31, 2015

Oh, I don't know UltimateAndroidNormal is deprecated, so I change it instead of UltimateAndroidGradle. I found the same files are also in UltimateAndroidGradle, so you can apply the same conversions for UltimateAndroidGradle.

For ProgressMenuItemActivity, the receiver should be put in onCreateOptionsMenu. I commit this change in 0cb7d85 and now it works.

@cymcsg cymcsg merged commit 0cb7d85 into cymcsg:dev Jun 1, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cymcsg

This comment has been minimized.

Show comment
Hide comment
@cymcsg

cymcsg Jun 1, 2015

Owner

Thank you very much.I have merged the PR.I am hoping for your more contribution to the project.

Owner

cymcsg commented Jun 1, 2015

Thank you very much.I have merged the PR.I am hoping for your more contribution to the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment