I'm adding a bunch of annotations to a map and as the user moves and pans around to different countries I remove the annotations and add in some more. The problem I'm facing is the new annotations don't show until I've interacted with the map, either tap, pinch, pan or zoom.

I've tried placing the map.addAnnotations() into a DispatchQueue but that didn't work and I'm also offsetting the built method loadNewCountry(country: String) into a dispatchGroup. None of these are working!

Note: I've got several thousand annotations of varying types so loading them all in memory won't work for older devices :)

func mapView(_ mapView: MKMapView, regionDidChangeAnimated animated: Bool) {
    
    checkIfLoadNewCountry()
}

func checkIfLoadNewCountry() {
    let visible  = map.centerCoordinate
    geocode(latitude: visible.latitude, longitude: visible.longitude) { placemark, error in
        if let error = error {
            print("\(error)")
            return
        } else if let placemark = placemark?.first {
            if let isoCountry = placemark.isoCountryCode?.lowercased() {
                self.loadNewCountry(with: isoCountry)
            }
        }
    }
}

func loadNewCountry(with country: String) {
    let annotationsArray = [
        self.viewModel1.array,
        self.viewModel2.array,
        self.viewModel3.array
        ] as [[MKAnnotation]]
    
    let annotations = map.annotations
    
    autoreleasepool {
        annotations.forEach {
            if !($0 is CustomAnnotationOne), !($0 is CustomAnnotationTwo) {
                self.map.removeAnnotation($0)
            }
        }
    }
    
    let group = DispatchGroup()
    let queue = DispatchQueue(label: "reload-annotations", attributes: .concurrent)
    
    group.enter()
    queue.async {
        self.viewModel1.load(country: country)
        group.leave()
    }
    
    group.enter()
    queue.async {
        self.viewModel2.load(country: country)
        group.leave()
    }
    
    group.wait()
    
    DispatchQueue.main.async {
        for annoArray in annotationsArray {
            self.map.addAnnotations(annoArray)
        }
    }
    
}

CodePudding user response：

The key issue is that the code is initializing the [[MKAnnotation]] with the current view model results, then starting the load of the view models models for a new country, and then adding the old view model annotations to the map view.

Instead, grab the [[MKAnnotation]] after the reloading is done:

func loadNewCountry(with country: String) {
    let annotations = map.annotations

    annotations
        .filter { !($0 is CustomAnnotationOne || $0 is CustomAnnotationTwo || $0 is MKUserLocation) }
        .forEach { map.removeAnnotation($0) }

    let group = DispatchGroup()
    let queue = DispatchQueue(label: "reload-annotations", attributes: .concurrent)

    queue.async(group: group) {
        self.viewModel1.load(country: country)
    }

    queue.async(group: group) {
        self.viewModel2.load(country: country)
    }

    group.notify(queue: .main) {
        let annotationsArrays: [[MKAnnotation]] = [
            self.viewModel1.array,
            self.viewModel2.array,
            self.viewModel3.array
        ]

        for annotations in annotationsArrays {
            self.map.addAnnotations(annotations)
        }
    }
}

Unrelated to the problem at hand, I have also:

• simplified the DispatchGroup group syntax;
• eliminated the wait as you should never block the main thread;
• eliminated the unnecessary autoreleasepool;
• added MKUserLocation to the types of annotations to exclude (even if you're not showing the user location right now, you might at some future date) ... you never want to manually remove MKUserLocation or else you can get weird UX;
• renamed annotationArrays to make it clear that you’re dealing with an array of arrays.

As an aside, the above raises thread-safety concerns. You appear to be updating your view models on a background queue. If you are interacting with these view models elsewhere, make sure to synchronize your access. And, besides, the motivating idea of “view models” (as opposed to a “presenter” pattern, for example) is that you hook them up so that they inform the view of changes themselves.

So, you might consider:

• Give the view models asynchronous startLoad methods;
• Give the view models some mechanism to inform the view (on the main queue) of changes when a load is done (whether observers, delegate protocol, closures, etc.).
• Make sure the view models synchronize interaction with their properties (e.g., array).

E.g., let us imagine that the view model is updating the view via closures:

typealias AnnotationBlock = ([MKAnnotation]) -> Void

protocol CountryLoader {
    var didAdd: AnnotationBlock? { get set }
    var didRemove: AnnotationBlock? { get set }
}

class ViewModel1: CountryLoader {
    var array: [CustomAnnotationX] = []
    var didAdd: AnnotationBlock?
    var didRemove: AnnotationBlock?

    func startLoad(country: String, completion: (() -> Void)? = nil) {
        DispatchQueue.global().async {
            let newArray: [CustomAnnotationX] = ...   // computationally expensive load process here (on background queue)

            DispatchQueue.main.async { [weak self] in
                guard let self = self else { return }

                self.didRemove?(self.array)           // tell view what was removed
                self.array = newArray                 // update model on main queue
                self.didAdd?(newArray)                // tell view what was added
                completion?()                         // tell caller that we're done
            }
        }
    }
}

That is a thread-safe implementation that abstracts the view and view controller from any of the complicated asynchronous processes. Then the view controller needs to configure the view model:

class ViewController: UIViewController {
    @IBOutlet weak var map: MKMapView!

    let viewModel1 = ViewModel1()
    let viewModel2 = ViewModel2()
    let viewModel3 = ViewModel3()

    override func viewDidLoad() {
        super.viewDidLoad()

        configureViewModels()
    }

    func configureViewModels() {
        viewModel1.didRemove = { [weak self] annotations in
            self?.map?.removeAnnotations(annotations)
        }
        viewModel1.didAdd = { [weak self] annotations in
            self?.map?.addAnnotations(annotations)
        }

        ...
    }
}

Then, the “reload for country” becomes:

func loadNewCountry(with country: String) {
    viewModel1.startLoad(country: country)
    viewModel2.startLoad(country: country)
    viewModel3.startLoad(country: country)
}

Or

func loadNewCountry(with country: String) {
    showLoadingIndicator()

    let group = DispatchGroup()

    group.enter()
    viewModel1.startLoad(country: country) {
        group.leave()
    }

    group.enter()
    viewModel2.startLoad(country: country) {
        group.leave()
    }

    group.enter()
    viewModel3.startLoad(country: country) {
        group.leave()
    }

    group.notify(queue: .main) { [weak self] in
        self?.hideLoadingIndicator()
    }
}

Now that’s just one pattern. The implementation details could vary wildly, based upon how you have implemented your view model. But the idea is that you should:

• make sure the view model is thread-safe;
• abstract the complicated threading logic out of the view and keep it in the view model; and
• have some process whereby the view model informs the view of the relevant changes.