Correct architecture of iOS ViewController loading async (HealthKit) data

Question

The code below works 90% of the time but every so often I will return an error (i.e. self.hideAllStackViewsAndShowNoWorkoutsMessage() will get called) even though there are workouts to be loaded. I think it's a VC lifecycle timing issue but I can't find a flaw in my code?

MyViewController {
       override func viewWillAppear(_ animated: Bool) {
            loadAndSetWorkout() 
        }



   func loadAndSetWorkout() {
        WorkoutManager.loadMostRecentWorkout { (workout, error) in

                DispatchQueue.main.async {
                    self.refreshControl?.endRefreshing()

                    if let unwrappedWorkout = workout {
                        self.selectedWorkout = unwrappedWorkout
                    } else {
                        if let unwrappedError = error {
                            self.hideAllStackViewsAndShowNoWorkoutsMessage()
                            print("Error in LastWorkoutTVC loadMostRecentWorkout = \(unwrappedError)")
                        }
                    }
                }
            }

    }

}








                 class func loadMostRecentWorkout(handler: @escaping (HKWorkout?, WorkoutManagerError?) -> Void) {

        let workoutPredicate = HKQuery.predicateForWorkouts(with: .other)
        let sourcePredicate = HKQuery.predicateForObjects(from: HKSource.default()) //limit query to only this app
        let mostRecentPredicate = HKQuery.predicateForSamples(withStart: Date.distantPast, end: Date(), options: .strictStartDate)
        let compound = NSCompoundPredicate(andPredicateWithSubpredicates: [workoutPredicate, sourcePredicate, mostRecentPredicate])

        let sortDescriptor = NSSortDescriptor(key: HKSampleSortIdentifierEndDate, ascending: false)

        let query = HKSampleQuery(sampleType: HKObjectType.workoutType(), predicate: compound, limit: 1, sortDescriptors: [sortDescriptor]) { (query, samples, error) in

            if let unwrappedError = error {
                handler(nil, WorkoutManagerError.generalError(unwrappedError.localizedDescription))
                return //added this return
            }

            guard let samples = samples as? [HKWorkout] else {
                handler(nil, WorkoutManagerError.generalError("no samples in loadMostRecentWorkout"))
                return
            }

            guard let mostRecentWorkout = samples.first else {
                handler(nil, WorkoutManagerError.generalError("no first in samples in loadMostRecentWorkout"))
                return
            }
            handler(mostRecentWorkout, nil)

        }
        HealthStoreSingleton.sharedInstance.healthStore.execute(query)
    }

Answer 1

I'm just putting this here to show another design, seeing that I can't put code in a comment.

The semantics of your completion handler related code are essentially this:

If there's at least one sample, give it with no errors. Otherwise, give an error if we got an error, or give the error that there were no samples.

You have the "no samples" error, so we don't really need the "no first sample" error. It's unnecessary, so we can get rid of that.

A possible refactor might look something like this.

// We either give a sample, or some error.

if let samples = samples as? [HKWorkout], let first = samples.first {
    handler(first, nil)
} else {
    handler(nil, .generalError(error?.localizedDescription ?? "no samples"))
}

It's the same semantics more clearly expressed.

This isn't an answer, but since your handler will never be given a sample and an error, and you're getting an error when you know you have samples in your own workout data, then maybe the culprit is the predicate / query code?