Custom map annotations and clusters

Question

I registered a CustomMapAnnotationView and a ClusterView: the first one simply extends MKMarkerAnnotationView and overrides annotation this way:

override var annotation: MKAnnotation? {
 willSet {
 // CustomMapAnnotation is based on MKPointAnnotation
 if let c = value as? CustomMapAnnotation {
 clusteringIdentifier = "c"
 markerTintColor = UIColor.orange
 displayPriority = .defaultLow
 }
 }
 }

ClusterView is a MKAnnotationView that is used when annotations can be grouped.

 mapView.register(CustomMapAnnotationView.self, forAnnotationViewWithReuseIdentifier: MKMapViewDefaultAnnotationViewReuseIdentifier)
 mapView.register(ClusterView.self, forAnnotationViewWithReuseIdentifier: MKMapViewDefaultClusterAnnotationViewReuseIdentifier)

Now the code review part.

I use CustomMapAnnotationView and ClusterView this way:

func mapView(_ mapView: MKMapView, viewFor annotation: MKAnnotation) -> MKAnnotationView? {
 guard !(annotation is MKUserLocation) else {
 return nil
 }
 if(annotation is CustomMapAnnotation){
 let annotationID = "c"
 var annotationView: CustomMapAnnotationView?
 if let dequeuedAnnotationView = mapView.dequeueReusableAnnotationView(withIdentifier: annotationID) as? CustomMapAnnotationView {
 annotationView = dequeuedAnnotationView
 annotationView!.annotation = annotation
 }
 else
 {
 annotationView = CustomMapAnnotationView(annotation: annotation, reuseIdentifier: annotationID)
 annotationView?.rightCalloutAccessoryView = UIButton(type: .detailDisclosure)
 }
 if let annotationView = annotationView {
 annotationView.canShowCallout = true
 }
 return annotationView
 }else{
 let clusterID = "Cluster"
 var clusterView = mapView.dequeueReusableAnnotationView(withIdentifier: clusterID)
 if clusterView == nil {
 clusterView = ClusterView(annotation: annotation, reuseIdentifier: clusterID)
 } else {
 clusterView?.annotation = annotation
 }
 return clusterView
 }
 }

This code perfectly works or at least it does what I wanted it to do. But I'm not sure this is correct way to perform this action inside the body of this function. Any suggestion is appreciated.

Answer 1

A couple of observations:

1. You have code that says:

   var annotationView: CustomMapAnnotationView?
   if let dequeuedAnnotationView = mapView.dequeueReusableAnnotationView(withIdentifier: annotationID) as? CustomMapAnnotationView {
    annotationView = dequeuedAnnotationView
    annotationView!.annotation = annotation
   }
   else
   {
    annotationView = CustomMapAnnotationView(annotation: annotation, reuseIdentifier: annotationID)
    ...
   }
   

   In iOS 11 and later, the mapView.dequeueReusableAnnotationView(withIdentifier:for:) can reduce that to a single line:

   let annotationView = mapView.dequeueReusableAnnotationView(withIdentifier: annotationID, for: annotation)
   

2. Your mapView(_:viewFor:) is configuring the annotation views. I’d move that configuration code into the MKAnnotationView implementations for better separation of responsibilities and eliminating cruft from your view controller. For example:

   class CustomMapAnnotationView: MKMarkerAnnotationView {
    static let clusteringIdentifier = Bundle.main.bundleIdentifier! + ".c"
    override init(annotation: MKAnnotation?, reuseIdentifier: String?) {
    super.init(annotation: annotation, reuseIdentifier: reuseIdentifier)
    clusteringIdentifier = CustomMapAnnotationView.clusteringIdentifier
    markerTintColor = .orange
    displayPriority = .defaultLow
    collisionMode = .circle
    rightCalloutAccessoryView = UIButton(type: .detailDisclosure)
    }
    required init?(coder aDecoder: NSCoder) {
    fatalError("init(coder:) has not been implemented")
    }
    override var annotation: MKAnnotation? {
    willSet {
    clusteringIdentifier = CustomMapAnnotationView.clusteringIdentifier
    }
    }
   }
   

   And

   class ClusterView: MKMarkerAnnotationView {
    override init(annotation: MKAnnotation?, reuseIdentifier: String?) {
    super.init(annotation: annotation, reuseIdentifier: reuseIdentifier)
    displayPriority = .required
    collisionMode = .circle
    markerTintColor = .purple
    }
    required init?(coder aDecoder: NSCoder) {
    fatalError("init(coder:) has not been implemented")
    }
   }
   

   This cleans up your mapView(_:viewFor:) and keeps the annotation view configuration code where it belongs.

3. Consider your registration of these classes:

   mapView.register(CustomMapAnnotationView.self, forAnnotationViewWithReuseIdentifier: MKMapViewDefaultAnnotationViewReuseIdentifier)
   mapView.register(ClusterView.self, forAnnotationViewWithReuseIdentifier: MKMapViewDefaultClusterAnnotationViewReuseIdentifier)
   

   Note, you’re registering these identifiers, so your mapView(_:viewFor:) shouldn’t then use its own reuse identifiers of c and Cluster, respectively.

4. But the whole point of MKMapViewDefaultAnnotationViewReuseIdentifier, etc., in iOS 11 and later is so that you don’t have to implement mapView(_:viewFor:) at all. So you can comment out that whole routine (now that you’ve moved the annotation view configuration code to the annotation views, themselves, where it belongs).

   That having been said, sometimes you need a mapView(_:viewFor:). For example, maybe you need to support iOS versions prior to 11. Or perhaps you have more than three types of annotation types (your main custom annotation, your cluster annotation, and MKUserLocation). But then you wouldn’t register MKMapViewDefaultAnnotationViewReuseIdentifier as the reuse identifier. You’d register your own reuse identifier.

   If you do use your own reused identifier, I would have a static property in my annotation view with the preferred reuse identifier, rather than sprinkling hardcoded strings throughout my codebase. E.g.

   class CustomMapAnnotationView: MKMarkerAnnotationView {
    static let preferredReuseIdentifier = Bundle.main.bundleIdentifier! + ".customMapAnnotationView"
    ...
   }
   

   And

   mapView.register(CustomMapAnnotationView.self, forAnnotationViewWithReuseIdentifier: CustomMapAnnotationView.preferredReuseIdentifier)
   

   And

   extension ViewController: MKMapViewDelegate {
    func mapView(_ mapView: MKMapView, viewFor annotation: MKAnnotation) -> MKAnnotationView? {
    switch annotation {
    case let annotation as CustomMapAnnotation:
    return mapView.dequeueReusableAnnotationView(withIdentifier: CustomMapAnnotationView.preferredReuseIdentifier, for: annotation)
    ...
    }
    }
   }
   

   But that’s not needed in your case. You can remove mapView(_:viewFor:) entirely.