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

implement zip function #44

Closed
DanielAsher opened this issue Aug 11, 2015 · 6 comments
Closed

implement zip function #44

DanielAsher opened this issue Aug 11, 2015 · 6 comments

Comments

@DanielAsher
Copy link
Contributor

would be great to have a zip function that takes two tasks with different progress, value and error types and returns a task that returns a tuple of values.

I've implemented the function with the signature

public class func zip<P1, V1, R1, P2, V2, R2>(task1: Task<P1, V1, R1>, _ task2: Task<P2, V2, R2>) -> Task<(P1, P2), (V1, V2), (R1?, R2?)>

but unfortunately I'm unable to call it without the error:

Cannot invoke 'zip' with an argument list of type '(Task<Void, String, String>, Task<Void, String, String>)'
@DanielAsher
Copy link
Contributor Author

I posted a question documenting my problem and solution. I'd be glad to submit a pull request if it was deemed useful, though I have not tested (or thought too deeply) about the solution.

@inamiy
Copy link
Member

inamiy commented Aug 13, 2015

Hi @DanielAsher, thanks for your proposal on new zip feature.

At a first glance, I found your method interface is a bit too complicated
just because SwiftTask has to deal with 3 generic types <P, V, E> for each task.

To alleviate this issue, I have implemented zip-like method called Task.all(),
which can be used to combine any number of tasks by using array as Task.Value type
rather than using tuple.
Though not strictly typed, this method will hopefully solve some scenarios.

There are, however, some problems with this approach too, that it has to use the same <P, V, E> type for all tasks.
That means, there needs some sort of type conversion to zip different types of tasks into one,
e.g. Task<P, [Any], E>.

I will look into this issue more carefully...

@DanielAsher
Copy link
Contributor Author

Hi @inamiy ,

thank you once again for a wonderful framework. I figured out the problem with my implementation. please follow the link above to see the answer. As a result I implemented zip as a member function. This also simplifies the solution significantly. An example call site is:

listener?.zip(praise).success { (recognition, utterance) in
    self.machine?.handleEvent(.Complete)
}

Once again I have not fully implemented and tested the solution, but it is working within my project. Enabling progress handling needs to be thought about too. But I'd be happy to issue a pull request if you feel it is appropriate. The current code is below.

best wishes,

Daniel

    func zip<Progress2, Value2, Error2>(task2: Task<Progress2, Value2, Error2>) -> Task<(Progress, Progress2), (Value, Value2), (Error?, Error2?)> {

        return Task<(Progress, Progress2), (Value, Value2), (Error?, Error2?)> { progress, fulfill, reject, configure in

            var completedCount = 0
            var rejectedCount = 0
            let totalCount = 2

            let cancelAll : Void -> Void = {
                self.cancel()
                task2.cancel() 
            }

            let pauseAll : Void -> Void = {
                self.pause()
                task2.pause() 
            }

            let resumeAll : Void -> Void = {
                self.resume()
                task2.resume() 
            }

            self.success { (value: Value) -> Void in

                synchronized(self) 
                {
                    completedCount++

                    if completedCount == totalCount 
                    {
                        fulfill( (self.value!, task2.value!) )
                    }
                }
            }.failure { (error, isCancelled) -> Void in

                synchronized(self) 
                {
                    reject( error, nil )
                    cancelAll() 
                }
            }

            task2.success { (value: Value2) -> Void in

                synchronized(self) 
                {
                    completedCount++

                    if completedCount == totalCount 
                    {
                        fulfill( (self.value!, task2.value!) )
                    }
                }
            }.failure { (error, isCancelled) -> Void in

                synchronized(self) 
                {
                    reject( nil, error )
                    cancelAll() 
                }
            }

            configure.pause = { pauseAll() }
            configure.resume = { resumeAll() }
            configure.cancel = { cancelAll() }

        }.name("\(self.name) zipped with \(task2.name)")
    }
}

@inamiy
Copy link
Member

inamiy commented Aug 16, 2015

I roughly sketched zip() function in SwiftTask.swift#L669-L699 (feature/zip branch) using Task.all() method, also handling progress values properly.

I think this works pretty well, but there also needs some refactoring in Task.all() because current BulkProgress is not useful for zipping.
Let me take some more time to improve this.

@DanielAsher
Copy link
Contributor Author

Thanks for this. I also need a little time to figure this one out. Conversion to Task<Any, Any, Any>.all([t1, t2]) certainly changes the syntax. I'd be interested to understand if it changes the semantics or performance. I'll pull and check out the new Task.zip!

@karapigeon
Copy link

Thanks for this!!!

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

No branches or pull requests

3 participants