DispatchGroup logical workflow

Question

I am trying to implement DispatchGroup as follows, but if the first call returns true, then the second one returns false, then overall result will return false.

However, if the first call returns false, then the second one returns true, then overall result will return false which is not what I expected.

I want to return false, if any of the call returns false. How could I able to handle this issue?

 func storeInformation(id: String?, _ completion: @escaping (Bool) -> ()) {
    guard
      let id =  id
    else {
      completion(false)
      return
    }
    let dispatchGroup = DispatchGroup()
    var groupResult: Bool = false
    dispatchGroup.enter()
    storeFeatures { success in
      if success {
        groupResult = true
      } else {
        groupResult = false
      }
      dispatchGroup.leave()
    }
    
    dispatchGroup.enter()
    storeClasses { success in
      if success {
        groupResult = true
      } else {
        groupResult = false
      }
      dispatchGroup.leave()
    }
    dispatchGroup.notify(queue: .main) {
      completion(groupResult)
    }
  }

  private func storeClasses(_ completion: @escaping(Bool) -> Void) {
    postClasses { (error) in
      if let _ = error {
        completion(false)
      } else {
        completion(true)
      }
    }
  }

  private func storeFeatures(_ completion: @escaping(Bool) -> Void) {
    postFeatures { (error) in
      if let _ = error {
        completion(false)
      } else {
        completion(true)
      }
    }
  }

Answer 1

If we look at your storeClasses and storeFeatures, we see that they are not really actions that return a Bool; they are inherently attempts to post something that can fail. Hence what you really want to know is not whether something returned true or false but whether or not it failed. That is what you really mean — and it is always better, in programming, to say what you mean.

Using the Combine framework, we can express that sort of behavior with unbelievable succinctness. When we have multiple asynchronous actions to perform simultaneously, that is a Merge. And if one of them fails, the entire Merge fails. In other words, the very thing you want to do is effectively automatic!

Imagine, for example, that we have expressed your post actions by wrapping them in deferred Futures of type <Void,Error>. And suppose we have methods storeClassesFuture and storeFeaturesFuture that produce those Futures. Then all you have to say is:

Publishers.Merge(storeClassesFuture(), storeFeaturesFuture())

That is literally all there is to it! If you subscribe to that Merge with a sink, then either it receives a finished completion or a failure completion. And guess what? It receives the failure completion if and only if one or both of the post actions failed! It receives the finished completion only if they both succeeded, which is exactly what you want to know.

As a test bed, here's a sample implementation of your storeInformation (I'm ignoring the String for purposes of the example):

var storage = Set<AnyCancellable>()
enum Oops : Error { case darn }
func storeInformation() {
    Publishers.Merge(storeClassesFuture(), storeFeaturesFuture())
        .receive(on: DispatchQueue.main)
        .sink { (completion) in
            switch completion {
            case .failure: print("at least one of them failed")
            case .finished: print("they both succeeded")
            }
            print("---")
        } receiveValue: { _ in }
        .store(in: &storage)
}

And just to act as a random test, here are two futures that can randomly succeed or fail:

func storeClassesFuture() -> AnyPublisher<Void,Error> {
    Deferred {
        Future<Void,Error> { promise in
            if Bool.random() {
                print("storeClassesFuture succeeded")
                promise(.success(()))
            } else {
                print("storeClassesFuture failed")
                promise(.failure(Oops.darn))
            }
        }
    }.eraseToAnyPublisher()
}
func storeFeaturesFuture() -> AnyPublisher<Void,Error> {
    Deferred {
        Future<Void,Error> { promise in
            if Bool.random() {
                print("storeFeaturesFuture succeeded")
                promise(.success(()))
            } else {
                print("storeFeaturesFuture failed")
                promise(.failure(Oops.darn))
            }
        }
    }.eraseToAnyPublisher()
}

And here's some sample output from calling storeInformation repeatedly:

storeClassesFuture succeeded
storeFeaturesFuture succeeded
they both succeeded
---
storeClassesFuture failed
storeFeaturesFuture failed
at least one of them failed
---
storeClassesFuture failed
storeFeaturesFuture succeeded
at least one of them failed
---
storeClassesFuture failed
storeFeaturesFuture failed
at least one of them failed
---
storeClassesFuture failed
storeFeaturesFuture succeeded
at least one of them failed
---
storeClassesFuture succeeded
storeFeaturesFuture succeeded
they both succeeded
---
storeClassesFuture succeeded
storeFeaturesFuture succeeded
they both succeeded
---
storeClassesFuture failed
storeFeaturesFuture succeeded
at least one of them failed
---
storeClassesFuture failed
storeFeaturesFuture succeeded
at least one of them failed
---
storeClassesFuture succeeded
storeFeaturesFuture succeeded
they both succeeded
---

As you can see, the logic you're after is perfectly expressed by the Merge of two failable Futures.

(This sort of thing is a very good reason to adopt the Combine framework instead of using DispatchGroup. I find that everything I used to do with DispatchGroup can be done better with Combine. This just happens to be a particularly clearcut instance.)

Answer 2

You have an "AND" semantic here, so you should write that in your code:

let dispatchGroup = DispatchGroup()
var groupResult: Bool = true // identity for AND
dispatchGroup.enter()
storeFeatures { success in
  groupResult = groupResult && success // here!
  dispatchGroup.leave()
}

dispatchGroup.enter()
storeClasses { success in
  groupResult = groupResult && success // and here
  dispatchGroup.leave()
}
dispatchGroup.notify(queue: .main) {
  completion(groupResult)
}

When each task finishes, you want to express the idea that

The group result should be true iff the previous group result is true AND success is true