Skip to content
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

Provide a ScheduledExecutorService to AsyncRequestQueue/AsyncNetwork. #367

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

jpd236
Copy link
Collaborator

@jpd236 jpd236 commented Sep 26, 2020

This is a subset of #363 which ensures a ScheduledExecutorService is available,
but does not yet define a new RetryPolicy interface. This gives us the
flexibility to introduce timer-based operations later without needing to change
the ExecutorFactory interface (which is now an abstract class to allow for future
additions, should any others be necessary).

This is a subset of google#363 which ensures a ScheduledExecutorService is available,
but does not yet define a new RetryPolicy interface. This gives us the
flexibility to introduce timer-based operations later without needing to change
the ExecutorFactory interface (which is now an abstract class to allow for future
additions, should any others be necessary).
* their needs. Apps must create ExecutorServices dynamically given a blocking queue rather than
* providing them directly so that Volley can provide a PriorityQueue which will prioritize
* requests according to Request#getPriority.
*/
public interface ExecutorFactory {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keeping the interface?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is taken from the Android API guidelines - since we can't use default interface methods in Volley, abstract classes are preferable because it gives us the flexibility to add methods in later versions. Since it seems like there's clearly value in such flexibility (as we're adding a scheduled Executor in this very PR), it seemed like a good time to make it an abstract class as well. I can't think of a concrete example of something we'd be likely to add here now that the scheduled executor is here, but I also don't see much benefit to keeping this an interface since I can't imagine anybody needs multiple inheritance to implement it.

@jpd236 jpd236 merged commit cde6d43 into google:master Sep 29, 2020
@jpd236 jpd236 deleted the scheduled-executor branch September 29, 2020 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants