[SWIFT] CollectionView, MapView et placement de cercle/image sur la map

Bonjour tout le monde,

Je crois que je m'y prend mal et je commence à tourner en rond, du coup je viens appeler à l'aide ^^

Je vais essayé d'expliquer le soucis simplement..

Je récupère via une API, des incidents que je dois afficher dans une collectionView

Voici à quoi doit ressembler ma view..

Si j'ai plusieurs incidents, je swipe de l'un à l'autre et les infos changent..
Jusque là, tout va bien sauf pour le cercle de couleur et l'image au centre du cercle.

Voici comment je m'y suis pris :

Pour créer le cercle et changer sa couleur, je passe par :

func mapView(_ mapView: MKMapView, rendererFor overlay: MKOverlay) -> MKOverlayRenderer

Arrivé dans cette fonction, je n'avais aucun moyen de savoir sur quel incident j'étais et les cercles/icones sont spécifique aux incidents.
Je me suis donc dit que j'allais créer une variable globale pour stocker cette info..

Donc dans mon code j'ai :

var TabInfosCercle : [String:String] = [:]

J'ai créer une class pour mes cellules de ma collectionView :

class collectionViewCelluleIncident : UICollectionViewCell {

     @IBOutlet weak var mapView: MKMapView!
     var TabListeOverlaysDejaEnPlace : [Int] = []
     // etc etc

      // rempli le template avec les infos reçues en JSON
      func displayContent(Incident: JSON, isSelected : Bool){
              self.labelAdresse.text = Incident["Adresse"].stringValue
              // etc etc

             // Pour ne pas ajouter plusieurs fois le cercle à chaque affichage de la map - DEBUT
             if(!TabListeOverlaysDejaEnPlace.contains(Incident["Id"].intValue)){ // Ne contient pas la valeur donc on l'ajoute
                  TabListeOverlaysDejaEnPlace.append(Incident["Id"].intValue) // ajoute au tableau des overlays déjà en place
                  let cercle = MKCircle(center: coordonnees, radius: rayon as CLLocationDistance)
                  self.mapView.addOverlay(cercle) // (MKMapViewDelegate)
              }
             // Pour ne pas ajouter plusieurs fois le cercle à chaque affichage de la map - FIN

             // stock l'info dans une variable globale et elle sera lu dans MKMapViewDelegate
             TabInfosCercle["Etat"] = Incident["Etat"].stringValue
             TabInfosCercle["IdTypeIncident"] = Incident["IdTypeIncident"].stringValue
      }
}

Ensuite, dans mon extension de MKMapViewDelegate :
Je lis la variable globale pour afficher la bonne couleur / le bon icône.

extension ListeIncidentsController: MKMapViewDelegate {

    // Pour changer la couleur du cercle en fonction de son état
    func mapView(_ mapView: MKMapView, rendererFor overlay: MKOverlay) -> MKOverlayRenderer {

        if overlay is MKCircle {
            let circle = MKCircleRenderer(overlay: overlay)

            switch(TabInfosCercle["Etat"]){
                case "0","1": // En attente, En cours
                    circle.strokeColor = UIColor.red
                    circle.fillColor = UIColor(hex: "#ff6a00", alpha: 0.4)
                break;

                case "2": // refusé
                    circle.strokeColor = UIColor.gray
                    circle.fillColor = UIColor(red: 0, green: 0, blue: 0, alpha: 0.2)
                break;

                case "3": // Terminé
                    circle.strokeColor = UIColor(hex: "#2aa532", alpha : 1)
                    circle.fillColor = UIColor(red: 0, green: 255, blue: 0, alpha: 0.2)
                break;

                default:
                    // normalement on ne doit jamais passer ici
                    circle.strokeColor = UIColor.blue
                    circle.fillColor = UIColor(red: 0, green: 255, blue: 0, alpha: 0.2)
                break;
            }

            circle.lineWidth = 1
            return circle
        }
        else {
            // Autre chose qu'un cercle
            return MKPolylineRenderer()
        }
    }

    // Pour changer l'icone de l'incident en fonction de l'incident
    func mapView(_ mapView: MKMapView, viewFor annotation: MKAnnotation) -> MKAnnotationView? {

        let identifier = "MyCustomPin"
        if annotation is MKUserLocation {
            return nil
        }

        var annotationView = mapView.dequeueReusableAnnotationView(withIdentifier: identifier)
        annotationView?.image = nil

        if annotationView == nil {
            annotationView = MKAnnotationView(annotation: annotation, reuseIdentifier: identifier)
            annotationView?.canShowCallout = true

            // On recupère le bon icon incident en fonction du type d'incident puis de l'état de l'incident
            var iconeIncident = "incident_Autre_rond_"

            switch(TabInfosCercle["IdTypeIncident"]){

                case "1":
                    iconeIncident = "incident_Incendie_rond_"
                break;

                case "2":
                    iconeIncident = "incident_Attentat_rond_"
                break;

                case "3":
                    iconeIncident = "incident_Inondation_rond_"
                break;

                case "4":
                    iconeIncident = "incident_Explosion_rond_"
                break;

                case "5":
                    iconeIncident = "incident_Intrusion_rond_"
                break;

                case "6":
                    iconeIncident = "incident_Autre_rond_"
                break;

                case "7":
                    iconeIncident = "incident_PanneElec_rond_"
                break;

                case "8":
                    iconeIncident = "incident_Incident_IT_rond_"
                break;

                default:
                    iconeIncident = "incident_Autre_rond_"
                break;
            }

            var couleurIcon = "gris"
            switch(TabInfosCercle["Etat"]){

                case "0","1": // En attente, En cours
                    couleurIcon = "orange"
                break;

                case "2": // refusé
                    couleurIcon = "gris"
                break;

                case "3": // Terminé
                    couleurIcon = "vert"
                break;

                default:
                    couleurIcon = "gris"
                break;
            }

            annotationView?.image = UIImage(named: iconeIncident+couleurIcon)

        } 
        else {
            annotationView?.annotation = annotation
            annotationView?.image = nil
        }

        return annotationView
    }
}

Du coup, j'ai l'impression que ça ne fonctionne pas comme je le pensais..
Pour moi, à chaque fois que j'affiche une cellule de ma collectionView, je stock dans la variable globale les infos concernant l'incident et lorsqu'il passe dans l'extension MKMapViewDelegate, autant pour l'overlay que pour l'annotation, il lis la variable globale et affiche la bonne couleur, le bon icône.
Manifestement, ça ne fonctionne pas comme ça ou alors j'ai loupé un truc..

En fait, ça affiche bien le premier comme il faut mais dès que je swipe sur les autres cellules, tout se mélange et j'ai l'impression qu'il s'y perd..
Il s'y perd tellement que je me retrouve avec des icônes et des cercles pas de la bonne couleur..
Du coup, je me dis que c'était peut-être pas la meilleure idée de passer par a variable globale, ça ne fonctionne peut-être pas comme je le pensais..

Comment faire pour passer les bonnes valeurs de mon incident aux fonctions de mon extension de MKMapViewDelegate ?

Réponses

  • Personne n'a une idée pour me débloquer ? :/

  • CéroceCéroce Membre, Modérateur
    juillet 2019 modifié #3

    Je pense que le vrai problème de ton architecture est que le view controller est le MKMapViewDelegate, alors que ça devrait être la cellule, puisque c'est elle qui contient la mapView. C'est pour cela que ça se mélange.
    D'autant plus qu'à tout moment il n'y a jamais plus de deux cellules à l'écran, alors elle vont être recyclées.

    Par ailleurs, interpréter du JSON n'est pas le rôle d'un objet de la couche Vue; donc pas celui de la cellule, ni celui du view controller.
    Le parsing du JSON aurait déjà du être fait en amont et stocké dans des objets Modèle. Par exemple, à l'issue de l'interprétation, tu devrais avoir une AccidentList qui contient des Accidents, avec un .type décrit sous forme d'enum.
    Ainsi, le datasource pour la collection view (le view controller) n'aurait plus qu'à passer un seul Accident à chaque cellule.

    Du coup, je me dis que c'était peut-être pas la meilleure idée de passer par a variable globale, ça ne fonctionne peut-être pas comme je le pensais..

    Ce n'est jamais une bonne idée d'utiliser une variable globale. Le problème étant qu'elle est accessible de n'importe où à n'importe quel moment, alors on n'a aucune idée de qui a écrit dedans et quand.

  • J'avais regardé rapidement, mais oui, c'est un poste assez long qui révèle une architecture à revoir, et je n'ai pas le temps en ce moment de faire une analyse plus poussée.

  • Merci pour vos réponses même si ça reste encore un peu flou pour moi.. mais on va y arriver petit à petit ^^

    Je pense que le vrai problème de ton architecture est que le view controller est le MKMapViewDelegate, alors que ça devrait être la cellule, puisque c'est elle qui contient la mapView

    Cette phrase par exemple est un peu flou..
    Le view controller est le MKMapViewDelegate ?

    Le parsing du JSON aurait déjà du être fait en amont et stocké dans des objets Modèle. Par exemple, à l'issue de l'interprétation, tu devrais avoir une AccidentList qui contient des Accidents, avec un .type décrit sous forme d'enum.
    Ainsi, le datasource pour la collection view (le view controller) n'aurait plus qu'à passer un seul Accident à chaque cellule.

    Le fait de stocker mon JSON dans des objets Modèle.. ok ça je pige.
    La dernière phrase me fait quand même tilté.. "la collection view n'aurait plus qu'a passer un seul Accident à chaque cellule"..
    C'est pas déjà ce que je fais avec le JSON ?
    A chaque cellule je passe qu'un Incident (JSON) nan ?

    // A chaque affichage d'une cellule
    func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "collectionViewCelluleIncident", for: indexPath) as! collectionViewCelluleIncident
        let Incident = TabListeIncidents[indexPath.section] // je recupère mon incident
        cell.displayContent(Incident: Incident, isSelected: cell.isSelected) 
        return cell
    }
    

    Là je passe bien qu'un seul incident en JSON pour moi.
    A moins qu'on ne parle pas de la même chose ?

  • CéroceCéroce Membre, Modérateur

    @Insou a dit :

    Je pense que le vrai problème de ton architecture est que le view controller est le MKMapViewDelegate, alors que ça devrait être la cellule, puisque c'est elle qui contient la mapView

    Cette phrase par exemple est un peu flou..
    Le view controller est le MKMapViewDelegate ?

    Oui, c'est dans ton code:

    extension ListeIncidentsController: MKMapViewDelegate
    

    C'est assez simple en fait, il faudrait que CollectionViewCelluleIncident se conforme à MKMapViewDelegate à la place. Il n'y aurait alors plus de mélange possible entre les différents Incidents. (ou plutôt Accidents, non ?)

    La dernière phrase me fait quand même tilté.. "la collection view n'aurait plus qu'a passer un seul Accident à chaque cellule"..
    C'est pas déjà ce que je fais avec le JSON ?
    A chaque cellule je passe qu'un Incident (JSON) nan ?

    Si effectivement, tu as raison. J'ai du mal à suivre le code dans ces conditions (ce n'est pas ta faute, un forum n'est pas trop adapté).

    P.S.: par convention, les noms de types doivent débuter par une majuscule alors que les variables par une minuscule.

  • CéroceCéroce Membre, Modérateur
    juillet 2019 modifié #7

    Et écourte tes méthodes aussi. Un exemple, ce code:

    var iconeIncident = "incident_Autre_rond_"
    switch(TabInfosCercle["IdTypeIncident"]){
    
        case "1":
            iconeIncident = "incident_Incendie_rond_"
        break;
    
        case "2":
            iconeIncident = "incident_Attentat_rond_"
        break;
    
        case "3":
            iconeIncident = "incident_Inondation_rond_"
        break;
    
        case "4":
            iconeIncident = "incident_Explosion_rond_"
        break;
    
        case "5":
            iconeIncident = "incident_Intrusion_rond_"
        break;
    
        case "6":
            iconeIncident = "incident_Autre_rond_"
        break;
    
        case "7":
            iconeIncident = "incident_PanneElec_rond_"
        break;
    
        case "8":
            iconeIncident = "incident_Incident_IT_rond_"
        break;
    
        default:
            iconeIncident = "incident_Autre_rond_"
        break;
    }
    

    Tu peux l'extraire dans une autre méthode:

    func iconName(for accidentType: String) -> String {
        switch(accidentType) {
            case "1":   return "incident_Incendie_rond_"
            case "2":   return "incident_Attentat_rond_"
            case "3":   return "incident_Inondation_rond_"
            case "4":   return "incident_Explosion_rond_"
            case "5":   return "incident_Intrusion_rond_"
            case "6":   return "incident_Autre_rond_"
            case "7":   return "incident_PanneElec_rond_"
            case "8":   return "incident_Incident_IT_rond_"
    
            default:    return "incident_Autre_rond_"
        }
    }
    

    En Swift, on n'a pas besoin de break à chaque case.
    Evidemment, tu devrais avoir une enum au lieu des Strings.

  • InsouInsou Membre
    juillet 2019 modifié #8

    C'est assez simple en fait, il faudrait que CollectionViewCelluleIncident se conforme à MKMapViewDelegate à la place. Il n'y aurait alors plus de mélange possible entre les différents Incidents. (ou plutôt Accidents, non ?)

    Il suffit juste que je remplace ListeIncidentsController par CollectionViewCelluleIncident pour qu'il s'y conforme ?
    Je vais aller faire quelques tests de ce côté là ^^

    Si effectivement, tu as raison. J'ai du mal à suivre le code dans ces conditions

    Ouf, j'avais peur de dire une connerie et de me faire incendier :lol:

    Du coup, autre question, est-ce vraiment nécessaire de passer par un modèle ?
    Je pense que ça doit être plus "propre" pour relire le code car on sait que mon json va créer des "modèle incident" mais de mon point de vue de débutant, au final on passe toujours un élément (incident) pour en lire ses valeurs (Id, Description, etc etc).
    Est-ce qu'il y a un réel intérêt dans mon cas (je parle pas pour la relecture de mon code mais plutôt d'autre avantage que je ne vois pas, ex: rapidité, etc etc) ? Si oui, lequel/lesquels ?
    Parce que perso, je m'en fou un peu de lire mes données dans un json plutôt que dans un objet incident mais c'est peut-être parce que je n'en voit pas encore l'avantage ^^

    Et écourte tes méthodes aussi.

    J'suis pas fan du code sur une seule ligne.. mais j'suis encore moins fan de mon switch à rallonge.. allez hop, j'vais faire ça aussi au passage ^^

  • CéroceCéroce Membre, Modérateur

    @Insou a dit :smile:
    Il suffit juste que je remplace ListeIncidentsController par CollectionViewCelluleIncident pour qu'il s'y conforme ?

    Oui, ou pas loin ;-)

    Du coup, autre question, est-ce vraiment nécessaire de passer par un modèle ?

    Nécessaire, dans le sens «obligatoire», non. Mais il y a un moment où tu vas devoir programmer dans les règles de l'art sinon, rien ne marche. Tiens, par exemple, comme dans le cas présent :-)
    L'organisation Model-View-Controller n'est pas obligatoire, seulement, si on ne la suit pas, on a beaucoup de mal à organiser son code. Plus l'appli grossit, et moins elle est gérable.

    Je pense que ça doit être plus "propre" pour relire le code car on sait que mon json va créer des "modèle incident" mais de mon point de vue de débutant, au final on passe toujours un élément (incident) pour en lire ses valeurs (Id, Description, etc etc).
    Est-ce qu'il y a un réel intérêt dans mon cas (je parle pas pour la relecture de mon code mais plutôt d'autre avantage que je ne vois pas, ex: rapidité, etc etc) ? Si oui, lequel/lesquels ?

    Oui c'est plus rapide, parce qu'on va comparer deux entiers plutôt que deux Strings.
    Mais surtout, ça évite des bugs parce que tu t'es trompé dans les strings; une String n'est pas vraiment "typée" par principe. On peut y stocker un peu ce qu'on veut.
    Alors qu'avec une enum, impossible de te tromper, parce que le compilateur gueule si les types sont différents ou le case est inconnu.

    Parce que perso, je m'en fou un peu de lire mes données dans un json plutôt que dans un objet incident mais c'est peut-être parce que je n'en voit pas encore l'avantage ^^

    Un autre avantage est qu'en utilisant le protocole Codable, tu peux convertir le JSON en struct ou class presque sans code. Ça devient intéressant, là, non ?

    J'suis pas fan du code sur une seule ligne.. mais j'suis encore moins fan de mon switch à rallonge.. allez hop, j'vais faire ça aussi au passage ^^

    Surtout, je voulais que tu extraies la fonction. Essaie d'écrire des méthodes de 10 lignes maxi. C'est faisable, crois-moi.

  • @Céroce a dit :
    Essaie d'écrire des méthodes de 10 lignes maxi. C'est faisable, crois-moi.

    +100

  • @Insou a dit :
    Du coup, autre question, est-ce vraiment nécessaire de passer par un modèle ?
    Je pense que ça doit être plus "propre" pour relire le code car on sait que mon json va créer des "modèle incident" mais de mon point de vue de débutant, au final on passe toujours un élément (incident) pour en lire ses valeurs (Id, Description, etc etc).
    Est-ce qu'il y a un réel intérêt dans mon cas (je parle pas pour la relecture de mon code mais plutôt d'autre avantage que je ne vois pas, ex: rapidité, etc etc) ? Si oui, lequel/lesquels ?

    Comme l'a dit Ceroce. Outre le fait de parler de Modèle (de MVC ou d'autres archi), il y a toujours une faute de frappe, un Incident["IdTypeincdent"] par exemple sur une faute de frappe.
    L'avantage est également que si demain c'est renommé, que finalement, il faut que tu fasses Incident["id"]["typeIncident"], ou autre, tu n'as à le changer que dans le parsing au niveau de ton modèle. Pas dans toute ton app.
    C'est là, dans l'exemple du MVC, que c'est séparé et tu ne touches donc qu'au "M" et pas au "VC".
    Avantage également, rajouter des méthodes. C'est bête, mais si tu as besoin de lui rajouter des méthodes intéressantes, des lazy vars, etc, qui n'ont de sens que pour cet objet, bah c'est plus facile.
    Et comme dit également, tu profites du compilateur.

  • Merci pour vos explications, j'ai moins l'impression de suivre des indications "parce que c'est comme ça et pas autrement" ^^

    Il y a de bons arguments, ça m'a convaincu ^^
    Je vais commencé à mettre ça en place sur mon ListeIncidentsController, ça sera l'occasion de tester :)

    Oui, ou pas loin ;-)

    J'ai donc remplacé ListeIncidentsController par collectionViewCelluleIncident et j'ai mis le delegate de la map sur ma collectionView via le storyboard (c'est ce qui me parait le plus logique mais je ne suis pas sûr..)
    Du coup, j'ai du louper un truc parce que je ne passe plus dedans et j'ai plus aucun icône (custmo) et cercle sur la map..

    Je vais continuer mes tests..

  • Bon bah après plusieurs tests, je ne passe plus dans mon extension collectionViewCelluleIncident : MKMapViewDelegate
    Pourtant je pense avoir bien tout relié..
    J'ai du zapper un truc :/

    Une idée pour me débloquer ? ^^

  • Joanna CarterJoanna Carter Membre, Modérateur

    Ne mets pas le map delegate dans le list controller. Regardes mon article sur CellPresenters et penses de mettre le delegate là…

    https://joannamacdev.github.io/The-Cell-Presenter/

  • Ne mets pas le map delegate dans le list controller

    Bah c'est déjà ce que j'ai corrigé avec mon message juste avant le tient nan ?
    Le delegate n'est plus sur le ListeIncidentsController mais sur le collectionViewCelluleIncident

  • Un petit coup de pouce svp, je patauge là :#

  • Je reviens donc sur ce soucis que j'avais mis de côté..

    Je passe maintenant par un objet Incident au lieu d'utiliser directement le json..
    A la réception de mon json, je rempli donc mon objet incident et j'utilise bien cet objet quand je veux récupérer les données, etc etc, pas de soucis (et c'est vrai que ça structure mieux le code ^^)

    J'ai donc toujours le même soucis, je ne passe plus dans mon extension collectionViewCelluleIncident : MKMapViewDelegate
    J'en reviens donc à ce que disais @Larme

    Ne mets pas le map delegate dans le list controller

    Qu'est ce que tu as voulu dire par là ?
    J'ai essayé de comprendre ton lien mais c'est un peu trop avancé pour moi là :/

    Le delegate de ma mapView est relié à ma collectionView.. ce qui me parait logique (mais je sais que je me trompe sinon ça fonctionnerait :# )

    J'explique mon "logique" : la mapView est dans chaque cellule de ma class collectionViewCelluleIncident : UICollectionViewCell donc je me dis que c'est là que je dois le mettre..

    Bref, je m'emmêle les pinceaux.. une idée ? :smiley:

  • CéroceCéroce Membre, Modérateur

    Nous aussi tu nous embrouilles!

    Sommes-nous d'accord, que tu as une MapView par cellule?
    Si c'est bien le cas, alors CollectionViewCelluleIncident doit se conformer à MKMapViewDelegate. Et évidemment, il faut que la cellule soit le délégué de la MapView. Sinon, à quoi bon qu'elle se conforme au protocole ?

  • Sommes-nous d'accord, que tu as une MapView par cellule?

    Oui

    Si c'est bien le cas, alors CollectionViewCelluleIncident doit se conformer à MKMapViewDelegate

    Comment je fais ça ?
    Comme ça ?

    extension collectionViewCelluleIncident : MKMapViewDelegate {
    
        func mapView(_ mapView: MKMapView, rendererFor overlay: MKOverlay) -> MKOverlayRenderer {
        ...
        }
    }
    

    car ça c'est fait..

    Et évidemment, il faut que la cellule soit le délégué de la MapView

    C'est ce que j'ai fait là nan ?

    Merci de ton aide en tout cas :)

  • CéroceCéroce Membre, Modérateur

    Et évidemment, il faut que la cellule soit le délégué de la MapView

    @Insou a dit :
    C'est ce que j'ai fait là nan ?

    D'après ton schéma, nan.

  • Donc d'après mon schéma, c'est la map qui a comme délégué la cellule ? (l'inverse de ce que je devrais faire ?)

    J'ai essayé de lier le delegate dans l'autre sens (CollectionViewListeIncident vers MapView) mais ma CollectionViewListeIncidents à déjà le delegate et le datasource lié à mon controller..

  • Joanna CarterJoanna Carter Membre, Modérateur
    septembre 2019 modifié #22

    Comme je t'ai déjà dit - n'utilises ni le collectionViewController, ni la cellule comme délégué. Regardes mon article pour voir comment créer un CellPresenter, qui devrait être le délégué pour la MapView.

  • Comme je disais, ton article est un peu trop avancé pour moi :/

    Bref..

    J'ai ajouté mapView.delegate = self dans ma fonction displayContent

    ça fonctionne, problème résolu, merci tout le monde.

Connectez-vous ou Inscrivez-vous pour répondre.