Search code examples
swiftapi-design

Is it correct to call a Swift closure multiple times


I'm designing a Swift API where I define a protocol EventTransmitter with a method to transmit a batch of Events. Processing a batch of Events can end with a list of both success and failed events.

So any implementation of this protocol should call a completion with the events that failed and/or the events that succeeded.

public enum EventResult {

    /// When the event was transmitted correctly.
    case success([Event])

    /// When the event failed and should be discarded
    case failure([Event])
}

public protocol EventTransmitter {

    func transmit(events: [Event], completion: @escaping (EventResult) -> Void)
}

With the above protocol, whoever implements the protocol has to call the completion twice:

public class HTTPTransmitter: EventTransmitter {

    public func transmit(events: [Event], completion: @escaping (EventResult) -> Void) {

        let failedEvents =  ...
        let successEvents = ...

        completion(.failure(failedEvents))

        completion(.success(successEvents))
    }
}

And the usage would be

transmitter.transmit(events: events, completion: { result in
    switch result {
    case .success(let events):
        // handle success events
    case .failure(let events):
        // handle failed events
    }
})

Is it correct to require to call one completion closure several times?

Or would it be more appropriate to have different closures for each case:

public protocol EventTransmitter {

    typealias EventCallback = (([Event]) -> Void)

    func transmit(events: [Event], onSuccess: @escaping EventCallback, onFailure: @escaping EventCallback)
}

Solution

  • If you wanted to do something like this where a list of failed events and succeeded events can happen then I would probably combine them both into the same callback...

    A callback something like...

    typealias EventCallBack = (succeeded: [Event], failed: [Event]) -> ()
    

    Then you only need to call the callback once.

    In the case where you have an enum that captures the state of the callback then I'd suggest only making it run once.

    This is purely from an API point of view. What does it mean to deal with a success twice? Or three times? Or a success and then a failure compared to a failure then a success. etc...

    Of course, you CAN call it multiple times and there are places where you might want to. For instance, storing a closure and calling it in response to a user interaction or something.

    But for something like this where there is one request I would suggest calling it once with all the necessary information.

    Another possible alternative

    Maybe what you could do is incorporate the success and failure closure handling into the Event itself.

    That way if an event fails you can call event.failed() and if it succeeded... event.succeeded() or something.

    I don't know in this specific case if that is a good idea but it is certainly another option :D

    Suggested by vacawama

    You could also define your enum like...

    enum EventResult {
        case success(Event)
        case failed(Event)
    }
    

    And then make your callback like...

    ([EventResult]) -> ()
    

    That way you just call it once with a single array of all of the individual results.

    This would be useful if it is important for the order of results to be the same as the order of events.