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

Replace closure callback with Promise/Future #12

Open
williamhjcho opened this issue Jan 6, 2018 · 4 comments
Open

Replace closure callback with Promise/Future #12

williamhjcho opened this issue Jan 6, 2018 · 4 comments

Comments

@williamhjcho
Copy link
Collaborator

Why implement Promise/Future

Promise<Value> enables more functionalities for chaining and perpetuating operations than a simple (Result<Value>) -> Void callback by allowing

  • Multiple transformations: map, flatMap
  • Error catching and treating
  • Easier operation workload dispatching to other threads/queues

Basic snippet

Inspired by RxSwift, Future (from fp paradigms) and Promise (from js):

public final class Promise<Value> {
    let callback: (@escaping (PromiseEvent<Value>) -> Void) -> Void

    public init(_ callback: @escaping (@escaping (PromiseEvent<Value>) -> Void) -> Void) {
        self.callback = callback
    }

    public func work(_ callback: @escaping (PromiseEvent<Value>) -> Void) {
        self.callback(callback)
    }

    public func then(onFulfilled: ((Value) -> Void)? = nil,
                     onFailed: ((Error) -> Void)? = nil) {
        self.work { event in
            switch event {
            case .value(let value):
                onFulfilled?(value)
            case .error(let error):
                onFailed?(error)
            }
        }
    }

    public func map<T>(_ transform: @escaping (Value) throws -> T) -> Promise<T> {
        return Promise<T> { callback in
            self.work { event in callback(event.map(transform)) }
        }
    }

    public func flatMap<T>(_ transform: @escaping (Value) throws -> Promise<T>) -> Promise<T> {
        return Promise<T> { callback in
            self.work { event in
                switch event {
                case .value(let value):
                    do {
                        try transform(value).work(callback)
                    } catch {
                        callback(.error(error))
                    }
                case .error(let error):
                    callback(.error(error))
                }
            }
        }
    }
}

extension Promise {
    public convenience init(value: Value) {
        self.init { callback in callback(.value(value)) }
    }

    public convenience init(error: Error) {
        self.init { callback in callback(.error(error)) }
    }

    public static func just(_ value: Value) -> Promise<Value> {
        return Promise(value: value)
    }

    public static func error(_ error: Error) -> Promise<Value> {
        return Promise(error: error)
    }

    public static func empty() -> Promise<Void> {
        return Promise<Void>(value: ())
    }
}

Use examples

Simple async dispatching

// creates the Promise,
let somePromise = Promise<Int> { callback in
    DispatchQueue.main.asyncAfter(deadline: .now() + 10) {
        callback(.value(0))
    }
}

somePromise
    .map { $0 * $0 }
    // only starts the work when this function is called
    .work { event in
        switch event {
            case .value(let value): ...
            case .error(let error): ...
        }
     }

// another way of listening to the result
somePromise.then(onFulfilled: { value in ... }, 
                 onFailed: { error in ... })

Other ways of creating Promises

Promise.just(0)
Promise<Value>.error(SomeError)
Promise.empty() // of Void type

Similarities

Using RxSwift as a reference, a Promise is a Single Cold Observable.
It contains some work which will only be executed when someone subscribes to it
There should be ways of making them hot Observables, share subscriptions, combine with others (eg: zip) etc

But differently from RxSwift, Promises do not send a stream of multiple objects, just zero or one (Value or Error)
And it should be noted that it also doesn't have to deal with disposables and whatnot

References

RxSwift
khanlou/Promise

@ronanrodrigo
Copy link
Owner

I really like the promises approach, I'm just afraid that it might complicate Frisbee users when it comes to creating their tests. What do you think?

@williamhjcho
Copy link
Collaborator Author

Not sure, haven't defined the API yet, just the draft above
It does involve a little bit of stretching to other patterns, which newcomers could not be familiar with

But I think it's a worthwhile implementation just for the capability of encapsulating and chaining workloads and operations

If needed, there could be a separate target for testing/mocking

@ronanrodrigo
Copy link
Owner

ronanrodrigo commented Jan 10, 2018

I was thinking about this issue. And I guess that should be an extension of Frisbee. The reason is with that, we will increase the code dependency to Frisbee. For example:

// Is OK to require Frisbee here
import Frisbee

class MoviesRepository {
    let getRequest: GetRequestable
    init(getRequest: GetRequestable) {
        self.getRequest = getRequest
    }

    func allMovies() -> Promise<[Movie]> {
        return getRequest.get(url: "")
    }
}
// Here, the UIViewController must to import Frisbee. But could be avoided.
// It is necessary because MoviesRepository are returning a Promise.
import Frisbee

class MoviesViewController: UIViewController {
    func viewDidLoad() {
        moviesRepository.allMovies().then({
            // ...
        }) 
    }
}

Maybe my knowledge about Promises is to poor. But I think this could spread the Frisbee library in places where the user don't need. And with callbacks, although it is simpler, the user does not need to import the Frisbee.

@williamhjcho
Copy link
Collaborator Author

As in RxSwift and RxCocoa?
Where RxCocoa creates extensions on top for RxSwift mostly on UI?

This isn't an issue about Promises but more on maybe unnecessary imports

But either way, the Promise function overload can still coexist with the closure one, so this importwould not be needed

The main idea is to allow more fine-tuning of the request's response, in more concise operations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants