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

GCD chained arguments and returns take2 #6

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

josephlord
Copy link
Owner

@chriseidhof

I've got strongly typed chained calls with arguments and return types working without any damage to the existing original API. (duemunk#8)

Surprisingly (or maybe not to you) the someChainableAsyncObject.cancel() still works passing the correct type () into the function.

     func cancel(withValue:ReturnType) {
        isCancelled = true
        returnedValueOpt = withValue
    }

By forcing the setting of the return on cancel it ensures that there is a non-Optional value to wait on (unless the block's type was optional to start with) and the chain can continue.

It needs more syntactic sugar on the new API features (Beta 6 doesn't seem to like the closure after the call syntax for these flexibly typed closures) and arguments are required on AsyncPlus calls even if it is (). It also needs more tests, a cleanup and I need to think about the effect on the drop in replacement nature of this for the main Async library but I'm happy I've got this far.

@josephlord
Copy link
Owner Author

@chriseidhof @duemunk

Any views on this code. I think it fully works in terms of chaining arguments but I don't know how easy it would be to port back to the original Async version. If I do merge it then the original Async will no longer be a drop in replacement. It could probably do with more tests but I keep not getting round to it.

@chriseidhof
Copy link

It's a bit too much code to quickly look at, hopefully I'll have some more time soon, but quite busy at the moment, sorry!

@duemunk
Copy link

duemunk commented Jan 5, 2015

@josephlord Have you made any progress on this?

@josephlord
Copy link
Owner Author

It works but The test coverage isn't very good. Having cracked the problem my interest level dropped.

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.

3 participants