-
-
Notifications
You must be signed in to change notification settings - Fork 29
Closures called on main queue #17
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
Comments
The reason the callbacks are made in the main thread is that this library was written for cases where you need to import a (potentially) large CSV file and want to give the user some kind of progress feedback. That's what the onProgress and onFinish callbacks were designed for and since the UI should only be updated in the main thread, those blocks are called on the main thread. I don't see any adverse effects except for the above mentioned UI changes when you would fork this and remove the main thread calls. If you want to ensure that you have an up-to-date framework, feel free to make your change in your fork additive (and non-breaking) so I can merge your changes via a Pull Request and we have optional support for explicitly defining the thread on which the callbacks should be called. I imagine something like this: let importer = CSVImporter<[String]>(...)
importer.qosClass = .background
importer.startImporting(...).onProgress {
// called in background thread
}.onFinish {
// called in background thread
} By default it would then still make the calls in the main thread, except for if you have defined a qosClass, then it would use a global DispatchQueue of the given qos class. |
I think that's reasonable. I am importing a large CSV but I'd like to have all processing finished on the background thread where I import into my indexable data model. While I love your implementation I may end up rolling my own or using one of the other parsers. My need is slightly different than what you're addressing. Thanks for getting back to me. |
But the importing is done on a background thread and adding support for background threads in the callbacks is not a complex task, if you're not sure how to add that feature yourself I can do that. Others may need the feature, too. |
I just implemented such a feature and released version 1.6.0 with it included. See the end of the Asynchronous with Callbacks section in the README for usage. Your requested behavior should now be possible @rodericj. If you have any more questions feel free to ask. Closing this. |
Note that I've also just added a synchronously working import method and released version 1.7.0 with it. Refer to the docs here. |
I noticed that the closures for completion come back on the main queue. I was hoping to have the callbacks on a background queue but I don't see any infra for that in the code or documentation. Is there a reason for this? If I were to just remove the
DispatchQueue.main.async
would you anticipate any adverse effects?The text was updated successfully, but these errors were encountered: