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.

Sign up for GitHub

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

Jump to bottom

Be able to set the ExecutorService. #187

Closed
chrisjenx opened this issue Feb 28, 2013 · 4 comments
Closed

Be able to set the ExecutorService. #187

chrisjenx opened this issue Feb 28, 2013 · 4 comments
Labels
Feature

Comments

@chrisjenx
Copy link
Contributor

I used a shared ThreadService for my app which I will share amongst background modules (robospice being one of them).

I would be good if I could provide the ImageLoader with an already created ExecutorService
@nostra13
Copy link
Owner

nostra13 commented Mar 8, 2013

I'll think about it.

nostra13 added a commit that referenced this issue Mar 12, 2013
@nostra13
Issue #187 - Be able to set the ExecutorService.
d063841 
Introduced ImageLoader.taskExecutor(...) and
.taskExecutorForCachedImages(...). Introduced ImageLoader.destroy().
Removed ImageLoader.stop(). Refactored warnings.
@chrisjenx
Copy link
Contributor Author

Cool looks good.

@campnic
Copy link

campnic commented Mar 13, 2013

One comment on this, which I realize is a bit late, but the idea of injecting an Executor Service into a library seems a little shaky to me. DI is great, but an executor service is not something that I think should be shared by two non-related pieces of code. I can understand providing a configuration mechanism for the executor, as UIL does, but taking an external executor service seems like it will open up users to a lot of unexpected behavior. "Why are my images loading slow?" is now a much more difficult question to answer because we'll have to know all the users of the executor service and how those executors were created.

@chrisjenx
Copy link
Contributor Author

Agreed you are correct, I would make sure the JavaDoc is explicitly clear
about only set if you KNOW what you are doing.

My issue is if I have 3 modules using 3 Executor services, I have an
unknown amount of maintainable threads.

But making this modular and flexible is always good.

On 13 March 2013 14:51, Nick Campion notifications@github.com wrote:

One comment on this, which I realize is a bit late, but the idea of
injecting an Executor Service into a library seems a little shaky to me. DI
is great, but an executor service is not something that I think should be
shared by two non-related pieces of code. I can understand providing a
configuration mechanism for the executor, as UIL does, but taking an
external executor service seems like it will open up users to a lot of
unexpected behavior. "Why are my images loading slow?" is now a much more
difficult question to answer because we'll have to know all the users of
the executor service and how those executors were created.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/187#issuecomment-14845520
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@campnic @chrisjenx @nostra13