Discussion :

[maa]

Bonjour,

J'ai lu ce tutoriel afin de comprendre comment organiser mes classes en modèles, vues et contrôleurs. En essayant de mettre en place ce pattern, j'ai été confronté aux problèmes suivant:

J'ai une classe modèle qui possède une propriété collection que j'aimerai représenter d'une manière différente pour les besoin de l'interface. Lorsque cette propriété est modifié, un évènement est levé et le contrôleur averti, met à jour la propriété collection de la vue correspondante.
Jusqu'à la pas de problème, mais ma classe modèle possède aussi d'autres propriété que j'aimerais également afficher dans l'UI. Faut-il créer des propriété correspondante dans la vue ? Je suppose que oui car le binding ce fait sur la vue, mais cela implique que la vue contienne une référence au modèle et que chaque propriété de la vue renvoie la propriété correspondante du modèle et cela devient rapidement lourd à mettre en place...
Une autre solution serait de mapper (avec éventuellement un outil comme AutoMapper) les propriétés du modèle à celle de la vue au moment de la création de celle-ci par le contrôleur. L'avantage est que la vue n'a pas besoin de contenir une instance du modèle. L'inconvénient est qu'en settant les propriétés de la vue, le modèle n'est pas modifié.

Maintenant prenons ce cas simplifié.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
public class Model1
{
     public Model2 MyProperty {get; set;}
}
 
public class Model2
{
}

Si je veux créer une vue pour le Model1, je dois également créer une vue pour le Model2 de cette manière

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
 
public class View1
{
     private Model1 model;
     ...
     public View2 MyProperty {get; set;}
}
 
public class View2
{
     private Model1 model;
     ...
}

Comme les classe modèles sont plus ou moins toutes liés, je dois créer une vue pour chacune d'elle. Il devient alors très fastidieux de synchroniser les vues et les modèles. Et si ma classe View2 ne faisait finalement que renvoyer les propriétés de Model2, aurait-elle vraiment sa raison d'être ? Cela représenterait beaucoup de code pour pas grand chose...

Merci d'avance pour vos éclaircissements sur ces questions.

[Maniak]

J'ai une classe modèle qui possède une propriété collection que j'aimerai représenter d'une manière différente pour les besoin de l'interface. Lorsque cette propriété est modifié, un évènement est levé et le contrôleur averti, met à jour la propriété collection de la vue correspondante.

Si c'est le contrôleur qui met à jour la vue, ça me fait plus penser à du MVP que du MVC.

Mis à part ça, je ne suis pas sûr de bien voir le problème. Le fait que Model1.MyProperty fasse référence à Model2 n'est d'aucune importance pour View1.

D'ailleurs dans ton exemple, View2 ne devrait pas faire référence à Model2 ? Parce que là ça embrouille un peu :)

En suivant le principe de MVC tel que décrit par Martin Fowler, View1 ferait référence à Model1 et à son contrôleur (on va dire Controller1 :)

Un évènement lambda se déclenche sur View1, qui passe le contrôle à Controller1, qui fait des actions sur Model1, qui provoque une modification de Model1 (une collection, quelques propriétés diverses et variées, peu importe), qui déclenche un évènement observé par View1, qui récupère les données du modèle et fait ce qu'elle veut des nouvelles valeurs. Que ce soit un mapping direct ou quelque chose de plus évolué.

Donc oui, la vue doit avoir une référence sur le modèle et faire la correspondance de toutes les données nécessaires avec les éléments graphiques. C'est après tout son seul job :)

Si View2 fait référence à Model2 (ou même à Model1 d'ailleurs, peu importe), le même principe s'applique. Donc oui, là aussi, View2 doit s'occuper de synchroniser les données dont elle a besoin.

La synchronisation elle-même ne devrait pas être si fastidieuse que ça dans le sens où une vue n'a besoin que de faire la correspondance des données qui l'intéressent. Si une autre vue est liée au même modèle mais affiche autre chose, elle fait sa correspondance sur ces autres données. Toutes les synchronisations de vues seront déclenchées en même temps par l'évènement du modèle.

Si c'est cette partie-là qui te pose un problème, il manque peut-être quelques détails sur le projet pour le mettre en évidence. Peut-être des classes responsables de trop de choses ? Peut-être aussi que MVC n'est pas adapté à ce que tu veux faire ? (ça arrive, c'est pour ça qu'il y a quelques zillions de variantes :)


En tout cas, là comme ça, un modèle avec une collection et quelques propriétés, deux vues basées dessus qui font la synchro de ce qui les intéresse, ça ne semble pas anormal :)

[maa]

Merci pour ta réponse.

Si c'est le contrôleur qui met à jour la vue, ça me fait plus penser à du MVP que du MVC.

Alors peut être que je devrais appliquer le pattern MVP. Voici mon scenario:
Je suis en train de créer une petite application pour définir des jours de livraison pour des commandes. En base de données, il y a une liste de transporteurs et pour chaque transporteurs une liste de jours de livraison possible. Avec mon application, l'utilisateur doit pouvoir modifier la liste des jours de livraison par transport. J'aimerais pour cela représenter dans une datagrid tous les jours de la semaine et que l'utilisateur puisse cocher les jours de livraison possibles. En base de donnée, j'ai donc liste de jours de livraison possibles (si un jour n'est pas possible, il ne figure pas dans la table) et côté interface tous les jours sont présentés (si un jour n'est pas possible, il est présent mais décoché).
Je dois donc compléter la collection de la vue en me basant sur la collection du modèle. Où doit se faire se travail? Je pensais que c'était le rôle du contrôleur.

Mis à part ça, je ne suis pas sûr de bien voir le problème. Le fait que Model1.MyProperty fasse référence à Model2 n'est d'aucune importance pour View1.

Ce que je voulais dire c'est que View1 doit forcément posséder alors une collection de View2 et ça nous oblige donc à créer aussi une classe View2, même si une vue de Model2 n'était pas nécessaire... A moins que View2 puisse posséder une collection de Model2 ?

D'ailleurs dans ton exemple, View2 ne devrait pas faire référence à Model2 ? Parce que là ça embrouille un peu

Oui, erreur de copier-coller.

La synchronisation elle-même ne devrait pas être si fastidieuse que ça dans le sens où une vue n'a besoin que de faire la correspondance des données qui l'intéressent. Si une autre vue est liée au même modèle mais affiche autre chose, elle fait sa correspondance sur ces autres données. Toutes les synchronisations de vues seront déclenchées en même temps par l'évènement du modèle.

Ok mais ça fait quand même tout une série d'évenements à mettre en place...
Mais si par exemple, Model1 possède un propriété Name de type string que l'on veut afficher telle quelle dans l'UI. Laquelle des ces solutions doit-on mettre en place ?

- faire une propriété "Name" correspondante dans la vue et la synchroniser avec le modèle par évenement
- faire une propriété "Name" dans la vue qui renvoie simplement vers la proprité correspondante du modèle (get{return Model.Name;} set {Model.Name = value})
- ne pas faire de propriété Name dans la vue correspondante et faire le binding sur le path "Model.Name". Cela implique que Model soit exposé publiquement par la vue.

[Maniak]

Je dois donc compléter la collection de la vue en me basant sur la collection du modèle. Où doit se faire se travail? Je pensais que c'était le rôle du contrôleur.

Dans le cadre du MVC (toujours pour celui décrit par Fowler), le contrôleur réagit aux actions effectuées sur la vue pour agir sur le modèle. La vue réagit aux évènements du modèle et se sert de lui pour s'afficher.

Le contrôleur ignore toute la partie mise à jour de la vue. Tout repose exclusivement sur le mécanisme d'évènements entre la vue et le modèle.

Ce que je voulais dire c'est que View1 doit forcément posséder alors une collection de View2 et ça nous oblige donc à créer aussi une classe View2, même si une vue de Model2 n'était pas nécessaire... A moins que View2 puisse posséder une collection de Model2 ?

Collection de View2 == liste des jours affichés, c'est ça ?
Si la liste est simple, il n'y a peut-être pas besoin de faire une vue séparée pour chaque ligne. Si View1 expose le nécessaire pour que son contrôleur puisse réagir aux évènements sur le datagrid et savoir comment les répercuter sur le modèle, tu pourrais t'en tirer avec une simple collection d'objets au niveau de View1 qui serait alimentée à partir des données du modèle puis bindée avec le datagrid. La classe des objets en question serait un wrapper des objets 'jour de livraison' du domaine, adaptée pour l'affichage.

Sinon, si tu dois vraiment avoir une deuxième vue... entre une collection de View2 avec chacune une instance de Model2 et une seule View2 représentant la liste avec une collection de Model2... dans les deux cas ça revient un peu au même.

J'ai un peu de mal à faire la correspondance entre Model1/Model2 et ton scénario cela dit. Si Model1 permet de récupérer la liste des jours sélectionnés + un évènement quand cette liste change, ça ne suffirait pas à ta (ou tes) vue(s) pour se débrouiller ?

Tu aurais par exemple View1/Controller1 avec référence sur Model1, contenant une View2 et réagissant à tous les évènements utiles, autres que ceux concernant View2.
View2/Controller2 avec référence aussi sur Model1, contenant une collection de jours 'bindable', réagissant aux évènements de Model1 liés aux jours.

Chaque vue s'occupe de ses affaires, le modèle regroupe ce dont a besoin ce 'bloc'. Regroupé sur Model1 ou réparti sur Model1 et 2, ça c'est une autre question :)

Dans tous les cas, tu n'es pas obligé d'avoir une instance de modèle pour chaque jour. Vu que tu traites une collection de jours, il semble logique que le modèle te fournisse... bah une collection de jours :)

Le modèle n'est pas "l'objet à afficher". Ça peut être le cas si les traitements sont vraiment simples, mais ça correspond plutôt à une partie de la couche métier qui permet d'effectuer les traitements nécessaires (pour le contrôleur) et de récupérer les données utilisées pour l'affichage. Tu n'as pas forcément besoin d'avoir les propriétés Name & co directement sur Model1. Tu peux plutôt demander à Model1 de te renvoyer l'objet (ou la collection) à afficher. Et si cet objet est directement bindable dans la vue... bah faut étudier mais pourquoi pas.

Ok mais ça fait quand même tout une série d'évenements à mettre en place...
Mais si par exemple, Model1 possède un propriété Name de type string que l'on veut afficher telle quelle dans l'UI. Laquelle des ces solutions doit-on mettre en place ?

- faire une propriété "Name" correspondante dans la vue et la synchroniser avec le modèle par évenement
- faire une propriété "Name" dans la vue qui renvoie simplement vers la proprité correspondante du modèle (get{return Model.Name;} set {Model.Name = value})
- ne pas faire de propriété Name dans la vue correspondante et faire le binding sur le path "Model.Name". Cela implique que Model soit exposé publiquement par la vue.

Pas la 2è déjà, c'est le contrôleur qui est chargé de répercuter les actions (donc les modifications) sur le modèle.

La 3è rejoint grosso modo le principe d'obtenir du modèle un objet directement bindé dans l'interface. Que ce soit le modèle lui-même me titille un peu (il est là pour faire le boulot, pas pour se faire binder sauvagement :), mais on va dire que c'est de la famille. Dans ce cas-là, le mécanisme de databinding peut jouer le rôle de contrôleur. Après tout il est là pour ça. Réagir aux évènements de la vue et transmettre le traitement au modèle. Par contre ça s'éloigne du MVC 'pur'. Dans ce cas-là, ton contrôleur n'est plus le seul en charge de transmettre les actions au modèle. Moins pratique pour les tests unitaires notamment.

La 1ère est la version 'fait main', sans databinding, mais c'est le même principe. Tu mets la propriété Name dans la vue, tu l'alimentes à partir du modèle, et quand le modèle balance son évènement Updated, tu remets à jour ta propriété dans la vue. Dans l'autre sens, quand tu fais des modifications, la vue transmet l'action au contrôleur, qui récupère la valeur de la propriété Name (et des autres) et va dire au modèle de se mettre à jour. Et le cycle recommence.


Cette version-là façon MVP (du moins façon Passive View, la plus pratique pour les tests unitaires :), ça donne :
Tu mets la propriété Name dans la vue. Au chargement, le Presenter l'alimente à partir du modèle. Quand tu fais des modifications dans la vue, le Presenter récupère l'action (comme en MVC, évènement ou appel direct depuis la vue), va dire au modèle de faire son boulot, puis remet la vue à jour. Pas forcément besoin d'évènement du côté du modèle si le Presenter est la seule source de changement sur les données affichées. Il sait quels traitements ont été effectués vu que c'est lui qui les lance et il sait quelles parties de l'interface remettre à jour si nécessaire.

Dans le cas d'un objet renvoyé par le modèle et directement bindable dans l'interface, c'est pareil, c'est le Presenter qui récupère l'objet en question et qui le passe ensuite à la vue. Plus besoin de recopier les propriétés. Mais pour ça il faut vraiment que la structure de l'objet bindé corresponde bien à l'affichage. S'il y a des divergences, il faut faire l'adaptation.

La vue elle-même ne fait grosso modo plus rien. Elle n'a en particulier plus de référence vers le modèle (et pas forcément vers le Presenter non plus d'ailleurs). Et ça veut dire que tu peux tester l'intégralité de la couche de présentation en injectant une fausse vue et un faux modèle au Presenter, et en vérifiant qu'il fait correctement le pont dans les deux sens. Mais là on s'éloigne du sujet d'origine :)

[maa]

Oublions Model1, Model2, View1 et View2. Je les ai pris pour parler d'un cas général et puis après j'ai basculé sur mon problème particulier. C'est vrai que ça prête à confusion.
Voici, mon code actuel, rassemblé en une classe:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
public class Shipping
{
	//Collection fournies par les donnée de la bdd
	private ObservableCollection<DeliveryDay> _Days;
	public ObservableCollection<DeliveryDay> Days
	{
		get { return _Days; }
		set
		{
			_Days = value;
			AvailableDays = new ObservableCollection<DeliveryDay>();
			for (int i = 0; i < 7; i++)
			{
				var day = _Days.FirstOrDefault(y => y.Day == i) ??
									new DeliveryDay
									{
										Day = i,
										IsDeliveryPossible = false,
									};
				day.DeliveryPossibleChanged += delegate
				{
					if (day.IsDeliveryPossible)
						_Days.Add(day);
					else
						_Days.Remove(day);
				};
				AvailableDays.Add(day);
			}
		}
	}
 
	//collection destinée à être bindée à l'UI
	public ObservableCollection<DeliveryDay> AvailableDays {get; set;}
}

La question que je me posait était: si on met la collection AvailableDays dans une class ShippingView, est-ce que cette collection ne devrait pas plutôt être de type : ObservableCollection<DeliveryDayView>, même si à priori je n'ai pas besoin d'une classe DeliveryDayView qui ne serait qu'une copie du modèle avec une propriété "IsDeliveryPossible" en plus qui quand elle est setté lève un evenement "DeliveryPossibleChanged".

Si j'ai bien compris, en MVC ça donnerait donc

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
    public class ShippingModel : INotifyPropertyChanged
    {
        private ObservableCollection<DeliveryDay> _days;
        public ObservableCollection<DeliveryDay> Days
        {
            get { return _days; }
            set
            {
                _days = value;
                if(PropertyChanged!=null)
                    PropertyChanged(this, new PropertyChangedEventArgs("Days"));
            }
        }
 
        #region INotifyPropertyChanged Members
 
        public event PropertyChangedEventHandler PropertyChanged;
 
        #endregion
    }
 
    public class ShippingView
    {
        private ShippingModel model;
 
        public ShippingView(ShippingModel model)
        {
            this.model = model;
            model.PropertyChanged += new PropertyChangedEventHandler(model_PropertyChanged);
        }
 
        void model_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            AvailableDays = new ObservableCollection<DeliveryDay>();
            for (int i = 0; i < 7; i++)
            {
                var day = model.Days.FirstOrDefault(y => y.Day == i) ??
                                    new DeliveryDay
                                    {
                                        Day = i,
                                        IsDeliveryPossible = false,
                                    };
                day.DeliveryPossibleChanged += new EventHandler(day_DeliveryPossibleChanged);
                AvailableDays.Add(day);
            }
        }
 
        void day_DeliveryPossibleChanged(object sender, EventArgs e)
        {
            if (DeliveryDayChanged != null)
                DeliveryDayChanged(this, new DeliveryDayEventArgs(sender as DeliveryDay));
        }
 
        public ObservableCollection<DeliveryDay> AvailableDays { get; set; }
 
        public event EventHandler<DeliveryDayEventArgs> DeliveryDayChanged;
    }
 
    public class DeliveryDayEventArgs :EventArgs
    {
        public DeliveryDayEventArgs(DeliveryDay deliveryDay)
        {
            DeliveryDay = deliveryDay;
        }
 
        public DeliveryDay DeliveryDay { get; set; }
    }
 
    public class ShippingController
    {
        private ShippingModel model;
 
        public ShippingController(ShippingModel model)
        {
            this.model = model;
            View = new ShippingView(model);
            View.DeliveryDayChanged += new EventHandler<DeliveryDayEventArgs>(View_DeliveryDayChanged);
        }
 
        void View_DeliveryDayChanged(object sender, DeliveryDayEventArgs e)
        {
            DeliveryDay day = e.DeliveryDay;
            if (day.IsDeliveryPossible)
                model.Days.Add(day);
            else
                model.Days.Remove(day);
        }
 
        public ShippingView View { get; private set; }
    }

Le code est plus lourd à écrire et je doute que ce découpage soit vraiment utile. Sans compter que je n'ai pas fais figurer ici d'autres propriétés de ShippingModel à afficher dans l'UI et que je devrait réécrire dans la vue ainsi que toute la plomberie de synchronisation par événement... Et sans compter que je n'ai pas fait de DeliveryDayView, j'ai simplement ajouté la propriété IsDeliveryPossible et l'événement DeliveryPossibleChanged dans le modèle.

La classe des objets en question serait un wrapper des objets 'jour de livraison' du domaine, adaptée pour l'affichage.

C'est un peu ce que j'ai fait avec ShippingView non ?

Tu n'as pas forcément besoin d'avoir les propriétés Name & co directement sur Model1. Tu peux plutôt demander à Model1 de te renvoyer l'objet (ou la collection) à afficher. Et si cet objet est directement bindable dans la vue... bah faut étudier mais pourquoi pas.

Name & co sont des propriété de l'entité Shipping, mais elles sont a afficher pour représenter les shipping dans l'UI. Faut-il maintenant repercuter ses propriété dans shippingview et les synchroniser ?

Pas la 2è déjà, c'est le contrôleur qui est chargé de répercuter les actions (donc les modifications) sur le modèle.

Dommage c'est celui que je sentais le mieux parce que le moins lourd.

La 1ère est la version 'fait main', sans databinding, mais c'est le même principe.

Si il y a un binding entre Name de shippingview et l'UI mais il faut ensuite synchroniser le modèle et la vue en passant par le controleur quand cette dernière est modifiée. C'est ça que je trouve lourd.

la vue transmet l'action au contrôleur, qui récupère la valeur de la propriété Name (et des autres) et va dire au modèle de se mettre à jour.

euh tu veux dire que c'est le contrôleur qui va mettre à jour le modèle, non ?

Dans le cas d'un objet renvoyé par le modèle et directement bindable dans l'interface, c'est pareil, c'est le Presenter qui récupère l'objet en question et qui le passe ensuite à la vue. Plus besoin de recopier les propriétés. Mais pour ça il faut vraiment que la structure de l'objet bindé corresponde bien à l'affichage. S'il y a des divergences, il faut faire l'adaptation.

Je ne vois pas trop ce que tu veux dire par "objet renvoyé par le modèle", ni ce que passer à la vue signifie... Ca voudrait dire que shippingModel serait passé à shippingView par le presenter ? Je ne comprends pas trop le sens.

Sinon il me semble que j'ai bien compris la différence entre MVP et MVC mais je ne sais pas trop lequel est le mieux adapté à mon cas.

Merci pour tes explications et conseils.

[Maniak]

La question que je me posait était: si on met la collection AvailableDays dans une class ShippingView, est-ce que cette collection ne devrait pas plutôt être de type : ObservableCollection<DeliveryDayView>, même si à priori je n'ai pas besoin d'une classe DeliveryDayView qui ne serait qu'une copie du modèle avec une propriété "IsDeliveryPossible" en plus qui quand elle est setté lève un evenement "DeliveryPossibleChanged".

Après 3 révisions de ce paragraphe, je penche au final pour une classe DeliveryDayView oui :)

La notion de DeliveryDay 'possible' et l'évènement associé sont des concepts de la vue. Le modèle n'est pas concerné. Lui traite des jours de livraison, et un jour de livraison est un jour où il peut y avoir une livraison, logique.

La vue est la seule a avoir besoin de ce 'possible' comme information supplémentaire, donc il serait logique que tout se situe au même niveau.

Parallèlement, si tu ajoutes tout ça directement sur DeliveryDay, ça veut dire que n'importe quel code utilisant cette classe aura accès à cette notion de jour 'possible' et pourra potentiellement déclencher une réaction en chaîne via des gestionnaires d'évènement dont il n'aura pas connaissance. Ça peut rapidement devenir très embrouillé.

Si j'ai bien compris, en MVC ça donnerait donc

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

...

Ça y ressemble oui. À vue de nez, tu as un petit problème avec l'attachement sur l'évènement DeliveryPossibleChanged. Vu que la plupart des mêmes jours reviennent à chaque fois (il n'y en a qu'un d'ajouté/supprimé), tu attaches un nouveau gestionnaire en plus de ce qui était déjà en place. Donc ça déclencherait OnDayDeliveryPossibleChanged plusieurs fois pour chaque jour.

Si tu passes par une classe DeliveryDayView, tu peux simplement recréer complètement la collection AvailableDays à chaque fois. Autre avantage de la séparation :)

Après, le principal problème est lié à l'utilisation des évènements pour dicter l'exécution du programme. En lisant le code du modèle seul, on n'a pas la moindre idée de ce qui se passe quand PropertyChanged est déclenché. Idem du côté de la vue. Ce qui se passe quand DeliveryDayChanged se lance n'est pas visible.

Pour le modèle, il n'y a pas le choix, c'est fait exprès. Pour le contrôleur, l'option de permettre à la vue de l'appeler directement pourrait améliorer la lisibilité.

Le code est plus lourd à écrire et je doute que ce découpage soit vraiment utile.:roll: Sans compter que je n'ai pas fais figurer ici d'autres propriétés de ShippingModel à afficher dans l'UI et que je devrait réécrire dans la vue ainsi que toute la plomberie de synchronisation par événement...

Pour un traitement simple sur un projet simple, ça peut être de l'overkill oui, clairement :)

Quand on s'est habitué à séparer tout ça, on le fait quasi-automatiquement, mais à petite échelle ce n'est pas forcément justifié.

MVC & co gagnent en intérêt au fur et à mesure que vues et modèles gagnent en complexité (et en fonction de l'importance qu'on apporte aux tests unitaires aussi). À court terme ça fait plus de code à écrire et de la plomberie à mettre en place, mais à plus long terme ça permet de faire évoluer l'application bien plus facilement que si tous les traitements sont entassés au même endroit.

La classe des objets en question serait un wrapper des objets 'jour de livraison' du domaine, adaptée pour l'affichage.

C'est un peu ce que j'ai fait avec ShippingView non ?

Ce serait plutôt DeliveryDayView dans ce cas-là. Donc cf plus haut pour l'intérêt (ou non) d'avoir cette classe.

Name & co sont des propriété de l'entité Shipping, mais elles sont a afficher pour représenter les shipping dans l'UI. Faut-il maintenant repercuter ses propriété dans shippingview et les synchroniser ?

Yup.

Dommage c'est celui que je sentais le mieux parce que le moins lourd.

Cf plus haut aussi donc. Selon la taille et la complexité de l'application, tu n'as peut-être pas besoin de sortir l'artillerie lourde (et encore, ce n'est pas la version la plus complexe :). Ça fait toujours un bon entrainement cela dit.

Si il y a un binding entre Name de shippingview et l'UI mais il faut ensuite synchroniser le modèle et la vue en passant par le controleur quand cette dernière est modifiée. C'est ça que je trouve lourd.

C'est le prix à payer :)
De toute façon, à partir du moment où tu ne peux pas coller les objets du domaine directement dans l'interface, tu dois bien faire des synchros dans les deux sens à un endroit ou à un autre. Ça peut être en se reposant intégralement sur le databinding, ça peut être tout directement dans une même classe, ou ça peut suivre le principe de MVC, MVP, etc. Il faut le faire quelque part, mais il y a le choix dans la méthode à utiliser :)

Évidemment si tu peux coller les objets du domaine directement dans l'interface, tu peux simplifier beaucoup de choses. Mais c'est rare de pouvoir le faire... en espérant pouvoir garder le code propre.

euh tu veux dire que c'est le contrôleur qui va mettre à jour le modèle, non ?

Nope. Le contrôleur va dire au modèle de travailler un peu. D'ailleurs ça me rappelle que c'est justement un problème avec ta version dans ce post :)
ShippingController n'a pas à faire model.Days.Add ou model.Days.Remove. C'est au modèle de faire le travail. Le contrôleur est juste là pour dire quoi faire. Ça peut être en conservant le test et en faisant model.AddDay et model.RemoveDay, ou plus simplement en virant le test et en faisant model.ToggleDeliveryDay. Tout le traitement passe du côté du modèle.
Tu peux aussi faire model.EnableDay et model.DisableDay, et le modèle se chargera non seulement d'ajouter/supprimer le jour, mais aussi de s'assurer qu'il ne peut pas y avoir de doublon ou de suppression d'un jour inexistant. Histoire que si pour une raison ou une autre, tu appelles 2 fois la fonction d'activation du même jour, ça ne te fasse pas de bug sournois. Tout ça, c'est la responsabilité du modèle. Il doit gérer sa propre intégrité. Et le contrôleur n'a en aucun cas à savoir comment est implémenté le modèle.

C'est notamment une application du principe Tell, Don't Ask. Ce n'est pas au contrôleur de savoir que quand un jour devient possible, il faut l'ajouter à la collection Days du modèle. C'est la cuisine interne du modèle. Le contrôleur a juste besoin de savoir quelle méthode du modèle appeler pour gérer l'action 'un jour est devenu possible/impossible'.

Je ne vois pas trop ce que tu veux dire par "objet renvoyé par le modèle", ni ce que passer à la vue signifie... Ca voudrait dire que shippingModel serait passé à shippingView par le presenter ? Je ne comprends pas trop le sens.

Non non, c'est juste ce que tu fais déjà avec Days. C'est histoire que le modèle permette d'obtenir des objets 'du domaine' (comme DeliveryDay). Ils ont leur rôle, leur fonction, et ils sont indépendants de la couche d'interface (et de la couche d'accès aux données aussi, tant qu'à faire).

Pour la partie 'passer à la vue', c'est juste que dans le cas de MVP/Passive View, c'est le Presenter qui passerait Name, Days etc à la vue (au plus basique, il ferait view.Name = model.Name;)
Dans cette version-là, la synchro entre vue et modèle se fait dans le presenter. Dans les deux sens.

En MVC, c'est séparé. Et dans certaines variantes de MVP, où la vue est plus autonome, elle peut aussi se mettre à jour directement depuis le modèle.

C'est un gros boxon quoi :)

Sinon il me semble que j'ai bien compris la différence entre MVP et MVC mais je ne sais pas trop lequel est le mieux adapté à mon cas.

Là comme ça je dirais que c'est un peu au feeling. Il n'y a que toi qui peut choisir. Je peux juste te conseiller de passer par la page GUI Architectures de Martin Fowler vu qu'il décrit très bien les différentes approches. Regarde notamment les diagrammes de chaque pour voir comment se fait la communication et prends celle qui te parle le plus. Essaye de trouver celle avec laquelle tu es le plus confortable.

Peu importe la variante, à partir du moment où les responsabilités sont séparées, tu y gagnes. Après, chaque approche a ses avantages et inconvénients, et chaque développeur peut avoir plus ou moins d'affinités avec.

Personnellement j'ai un petit faible pour Passive View, mais ça ne veut pas dire que c'est la méthode la plus appropriée dans la plupart des cas :)

[maa]

L'application que je développe est très petite. En fait je cherche plutôt à m'entrainer et à comprendre les principe MVC et MVP, mais c'est clairement de l'artillerie lourde pour une application qui ne sera pas destinée à grossir...

Après, le principal problème est lié à l'utilisation des évènements pour dicter l'exécution du programme. En lisant le code du modèle seul, on n'a pas la moindre idée de ce qui se passe quand PropertyChanged est déclenché. Idem du côté de la vue. Ce qui se passe quand DeliveryDayChanged se lance n'est pas visible.

Pour le modèle, il n'y a pas le choix, c'est fait exprès. Pour le contrôleur, l'option de permettre à la vue de l'appeler directement pourrait améliorer la lisibilité.

Donc il ne faudrait pas communiquer par événement entre le vue et le contrôleur mais plutôt par appel directe de méthode. Cela implique que la vue contienne une instance du contrôleur et la lie donc à celui ci.

Nope. Le contrôleur va dire au modèle de travailler un peu. D'ailleurs ça me rappelle que c'est justement un problème avec ta version dans ce post

Dans ce cas le contrôleur ne sert plus à grand chose sauf éventuellement si il y a plusieurs vues ?
Dans le tutoriel dont je parle plus haut il est dit:

* Le modèle : Il représente les données de l'application. Il définit aussi l'interaction avec la base de données et le traitement de ces données.
* La vue : Elle représente l'interface utilisateur, ce avec quoi il interagit. Elle n'effectue aucun traitement, elle se contente simplement d'afficher les données que lui fournit le modèle. Il peut tout à fait y avoir plusieurs vues qui présentent les données d'un même modèle.
* Le contrôleur : Il gère l'interface entre le modèle et le client. Il va interpréter la requête de ce dernier pour lui envoyer la vue correspondante. Il effectue la synchronisation entre le modèle et les vues.

Finalement j'ai l'impression que le contrôleur ne sert que d'intermédiaire entre la vue et le modèle dans un sens (il ne fait que relayer une demande de mise à jour des données) et dans l'autre sens (modèle->vue) il n'est même pas consulté. Donc je ne vois pas trop en quoi il effectue une synchronisation.
Pour la vue, c'est dans mon cas elle qui s'occupe de mettre à jour AvailableDays en fonction de model.Day or celle-ci n'est pas censé effectuer de traitement.
D'ailleurs n'est-ce pas aussi un problème que le contrôleur ne soit pas consulté quand le modèle change et que les modifs doivent être répercutées dans les vues ? Parce que c'est lui qui décide de la ou les vue(s) utile(s) et toutes les autres vues seront alors inutilement mise à jour.

C'est histoire que le modèle permette d'obtenir des objets 'du domaine' (comme DeliveryDay). Ils ont leur rôle, leur fonction, et ils sont indépendants de la couche d'interface (et de la couche d'accès aux données aussi, tant qu'à faire).

Peux-tu m'explique ce que signifie "objet du domaine" ? Je pensais que DeliveryDay était un modèle (j'aurais dû l'appeler DeliveryDayModel mais comme je pensais aussi lui faire jouer le rôle de la vue...).

Merci encore pour tes conseils.

[Maniak]

Donc il ne faudrait pas communiquer par événement entre le vue et le contrôleur mais plutôt par appel directe de méthode. Cela implique que la vue contienne une instance du contrôleur et la lie donc à celui ci.

Yup. Il faut choisir entre ajouter une dépendance vue->contrôleur et gagner en compréhension de ce qui se passe quand on regarde le code de la vue, ou continuer de se baser sur des évènements. Les deux fonctionnent, c'est au choix.

Dans ce cas le contrôleur ne sert plus à grand chose :( sauf éventuellement si il y a plusieurs vues ?

C'est un des objectifs de la chose. Plusieurs vues qui ont chacune leur façon de se mettre à jour, et un contrôleur qui gère les actions. Chaque vue a un comportement similaire, centralisé dans le contrôleur, mais une apparence et des règles d'affichage différentes.

Dans le tutoriel dont je parle plus haut il est dit:

...

Pour la vue, ça correspond. Elle se contente de l'affichage, à partir de ce qui est fourni par le modèle. Pour le traitement, les actions sont simplement relayées au contrôleur.
Pour le contrôleur, il y a une petite divergence d'avec la description de Fowler. Dans sa description, rien n'indique que le contrôleur soit responsable de créer la ou les vues. Il y a un contrôleur, il est rattaché à une (ou plusieurs) vue(s), il est appelé quand une action est à effectuer et il fait travailler le modèle en fonction. Et donc, le contrôleur est totalement ignorant de la mise à jour de la vue puisque que tout se passe entre vue(s) et modèle, via évènements.

Finalement j'ai l'impression que le contrôleur ne sert que d'intermédiaire entre la vue et le modèle dans un sens (il ne fait que relayer une demande de mise à jour des données) et dans l'autre sens (modèle->vue) il n'est même pas consulté. Donc je ne vois pas trop en quoi il effectue une synchronisation.

Disons que ce n'est pas tellement une synchronisation mais plutôt une représentation du comportement de la vue. Si une action particulière doit se traduire en plusieurs appels du modèle, en fonction de certaines conditions, c'est le contrôleur qui s'en occupe.

Forcément, si chaque action sur la vue se traduit simplement en un appel d'une méthode du modèle, ça peut à nouveau paraître lourd pour pas grand chose. C'est aussi un cas où l'intérêt du contrôleur augmente avec la complexité du comportement des vues.

Pour la vue, c'est dans mon cas elle qui s'occupe de mettre à jour AvailableDays en fonction de model.Day or celle-ci n'est pas censé effectuer de traitement.

De traitement 'métier'. Mais elle est responsable du traitement d'affichage. Et vu qu'AvailableDays ne sert que pour l'affichage, ça tombe pile dans son panier.

D'ailleurs n'est-ce pas aussi un problème que le contrôleur ne soit pas consulté quand le modèle change et que les modifs doivent être répercutées dans les vues ? Parce que c'est lui qui décide de la ou les vue(s) utile(s) et toutes les autres vues seront alors inutilement mise à jour.

Le contrôleur peut observer le modèle comme les vues pour réagir à certains évènements, mais c'est dans le cas où un changement sur le modèle doive se traduire par une nouvelle action.
S'il n'y a pas de comportement particulier (autre qu'une mise à jour de l'affichage) lié à un changement sur le modèle, le contrôleur ne s'en préoccupe pas. C'est justement un des intérêts de cette séparation. Un contrôleur n'a pas besoin de savoir quoi que ce soit sur la façon dont les vues sont affichées, ni de savoir quelles vues sont affichées.

Toutes les vues qui surveillent les évènements du modèle sont notifiées, c'est le but. Ce n'est pas le contrôleur qui décide, c'est... disons l'entité supérieure qui crée les triplets modèle/vue/contrôleur, quelle qu'elle soit :)

À toi de faire en sorte que les évènements du modèle soient suffisamment fins pour que les vues qui ne s'intéressent pas à certains changements ne soient pas obligées d'en être informées. Ou au moins qu'elles puissent déterminer si elles ont besoin de se mettre à jour ou non. Au pire, elles se remettront à jour. Pas bien grave, à moins que ça pose un vrai problème de performance. À nouveau, c'est du traitement d'affichage, donc c'est intégralement la responsabilité des vues.

Peux-tu m'explique ce que signifie "objet du domaine" ? Je pensais que DeliveryDay était un modèle (j'aurais dû l'appeler DeliveryDayModel mais comme je pensais aussi lui faire jouer le rôle de la vue...).

C'est un effet d'avoir mis le doigt dans le DDD :)
Le domaine en question est le Domain model. La représentation des concepts du système développé et de leurs relations. DeliveryDay en fait clairement partie, ainsi que tout autre objet représentant une entité du système (la livraison elle-même, les clients, ...)
Le modèle de MVC est lié à ce domaine.

Pour embrouiller encore un peu le terme modèle, tu as aussi des ViewModel, comme utilisés en WPF, Silverlight et ASP.NET MVC. Là, c'est la famille des classes comme DeliveryDayView. Des représentations de concepts du domaine, adaptées pour l'affichage.

[maa]

Disons que ce n'est pas tellement une synchronisation mais plutôt une représentation du comportement de la vue. Si une action particulière doit se traduire en plusieurs appels du modèle, en fonction de certaines conditions, c'est le contrôleur qui s'en occupe.

certaines conditions sur la vue ou sur le modèle ? Peux-tu donner un exemple ?

Forcément, si chaque action sur la vue se traduit simplement en un appel d'une méthode du modèle, ça peut à nouveau paraître lourd pour pas grand chose. C'est aussi un cas où l'intérêt du contrôleur augmente avec la complexité du comportement des vues.

Oui c'est ce que je me dis... Car maintenant ma vue appelle directement une méthode ToogleDeliveryDay de mon controleur quand une deliverydayview est mise à jour. Mais la méthode controleur.ToogleDeliveryDay() ne fait que rappeler model.ToogleDeliveryDay(). Comme la vue possède une instance du modèle, elle pourrait appeler directement model.ToogleDeliveryDay(). Je ne vois pas trop l'intérêt de passer par le contrôleur dans mon cas.

Évidemment si tu peux coller les objets du domaine directement dans l'interface, tu peux simplifier beaucoup de choses. Mais c'est rare de pouvoir le faire... en espérant pouvoir garder le code propre.

donc le domaine contient les objets modèle, mais pas les vues et les contrôleurs, n'est-ce pas ? Et par "coller ces objets directement dans l'UI" tu veux dire les binder directement sans utiliser de vue et de contrôleur ?

Je me pose maintenant les questions suivantes. Faut-il une classe DeliveryDayControleur ? Si oui qui doit l'instancie ? ShippingView ? Qui instancie les DeliveryDayView ? Peut-on parfois se passer de contrôleurs ? (juste un modèle et une vue). Dans mon cas ShippingController est-il aussi nécessaire ?

[Maniak]

certaines conditions sur la vue ou sur le modèle ? Peux-tu donner un exemple ?

Bah si telle action a été demandée et que telle valeur est sélectionnée alors il faut lancer tel traitement sur le modèle, sinon tel autre. Bref un truc un peu plus évolué que "je prends ces infos et j'appelle cette méthode du modèle" :)

Comme la vue possède une instance du modèle, elle pourrait appeler directement model.ToogleDeliveryDay(). Je ne vois pas trop l'intérêt de passer par le contrôleur dans mon cas.

Moi non plus :)
Mais si tu veux faire du MVC, c'est ce que tu devrais faire.
L'intérêt c'est que si tu as d'autres actions plus complexes à gérer, la séparation sera déjà faite et tu n'auras pas besoin de mixer le traitement d'affichage et un traitement d'action complexe dans une même classe.
L'inconvénient, c'est qu'il faut passer par le contrôleur même juste pour une ligne, pour que la responsabilité soit située à un seul endroit.

Dans ton cas donc, ça en revient toujours au même choix. Si ton projet est simple, tu n'as pas forcément besoin de toute la mécanique de MVC. Mais si tu veux t'en servir comme entrainement, il faut t'attendre à avoir l'impression d'ajouter du code et de compliquer le design pour pas grand chose. Dans cette optique, l'intérêt du MVC est pour toi, pour la pratique. Pas forcément pour ce projet-ci, mais c'est un pas vers l'ajout d'un nouvel outil dans ta besace pour la suite :)

Pour MVC comme pour beaucoup d'autres choses, il vaut mieux pratiquer à petite échelle d'abord plutôt que d'essayer de l'appliquer directement dans un gros projet, en pondant au final un truc instable et maladroit parce que tu auras tatonné en essayant de voir comment ça fonctionne. C'est pas forcément glorieux, mais ça a son utilité :)

donc le domaine contient les objets modèle, mais pas les vues et les contrôleurs, n'est-ce pas ?

Rien ne les en empêche. Le domaine est en quelque sorte le coeur de l'application. Qu'il se retrouve un peu partout n'est pas anormal.

Et par "coller ces objets directement dans l'UI" tu veux dire les binder directement sans utiliser de vue et de contrôleur ?

La vue si. C'est elle qui gère cette partie-là, donc c'est elle qui prendrait un objet fourni par le modèle et le binderait directement à un contrôle (une collection de DeliveryDay bindée à un DataGrid plutôt qu'une collection de DeliveryDayView). Et là, si le databinding s'occupe de mettre à jour l'objet en question automatiquement, ça empiète sur les territoires à la fois du contrôleur (gestion de l'action de mise à jour) et du modèle (modification d'un élément du domaine). La séparation des responsabilités n'est plus aussi claire.

Je me pose maintenant les questions suivantes. Faut-il une classe DeliveryDayControleur ? Si oui qui doit l'instancie ? ShippingView ? Qui instancie les DeliveryDayView ? Peut-on parfois se passer de contrôleurs ? (juste un modèle et une vue). Dans mon cas ShippingController est-il aussi nécessaire ?

DeliveryDayView est juste un élément de présentation (donc affichage, donc responsabilité de la vue, ce qui devrait répondre à ta question sur qui doit l'instancier) qui fait partie de la vue. Il ne permet pas d'interactions complexes avec l'utilisateur qui donneraient un intérêt à en séparer la gestion dans une classe à part. D'autre part ce n'est ni un contrôle, ni un formulaire. C'est une adaptation d'un DeliveryDay pour assister à l'affichage. Ce n'est pas l'interface graphique elle-même.

On peut se passer de contrôleurs si la gestion du comportement de la vue est basique. Mais donc ce n'est plus du MVC :)

Et dans ton cas, techniquement, basé sur ce que tu as décrit dans ce topic, j'aurais tendance à dire que non, le contrôleur n'est pas utile. Les seules actions dont il a été question ici sont de simples délégations au modèle, donc bon...

En revanche si tu as d'autres actions plus complexes à gérer et/ou si tu veux pratiquer le MVC, alors oui, il te faut le contrôleur :)

[maa]

J'ai essayé de réécrire mon code en prenant en compte toutes tes remarques. Ça donne :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
 
    public class ShippingModel : INotifyPropertyChanged
    {
        private IList<DeliveryDayModel> _days;
        public IList<DeliveryDayModel> Days
        {
            get { return _days; }
            set
            {
                _days = value;
                if(PropertyChanged!=null)
                    PropertyChanged(this, new PropertyChangedEventArgs("Days"));
            }
        }
 
        #region INotifyPropertyChanged Members
 
        public event PropertyChangedEventHandler PropertyChanged;
 
        #endregion
 
        public void AddDeliveryDay(DeliveryDayModel day)
        {
            Days.Add(day);
        }
 
        public void RemoveDeliveryDay(DeliveryDayModel day)
        {
            Days.Remove(day);
        }
    }
 
    public class ShippingView
    {
        private ShippingModel model;
 
        private ShippingController controller;
 
        public ShippingView(ShippingModel model, ShippingController controller)
        {
            this.model = model;
            this.controller = controller;
            model.PropertyChanged += new PropertyChangedEventHandler(model_PropertyChanged);
        }
 
        private void model_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            AvailableDays = new ObservableCollection<DeliveryDayView>();
            for (int i = 0; i < 7; i++)
            {
                var dayModel = model.Days.FirstOrDefault(y => y.Day == i);
                var day = new DeliveryDayView(dayModel, DaysConverter.GetDayString(i));                    
                day.PropertyChanged += new PropertyChangedEventHandler(day_PropertyChanged);
                AvailableDays.Add(day);
            }
        }
 
        public void day_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            controller.ToogleDeliveryDay(sender as DeliveryDayView);
        }
 
        public ObservableCollection<DeliveryDayView> AvailableDays { get; set; }
    }
 
    public class ShippingController
    {
        private ShippingModel model;
 
        public ShippingController(ShippingModel model)
        {
            this.model = model;
            View = new ShippingView(model, this);
        }
 
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            DeliveryDayModel dayModel = dayView.Model;
            if(dayModel == null)
            {
                dayModel = new DeliveryDayModel(DaysConverter.GetDayInt(dayView.Day));
                dayView.Model = dayModel;
            }
            if(dayView.IsDeliveryPossible)
                model.AddDeliveryDay(dayModel);
            else
                model.RemoveDeliveryDay(dayModel);
        }
 
        public ShippingView View { get; private set; }
    }
 
    public class DeliveryDayModel : INotifyPropertyChanged
    {
        public DeliveryDayModel(int day)
        {
            _day = day;
        }
 
        private int _day;
        public int Day
        {
            get { return _day; }
            set
            {
                _day = value;
                OnPropertyChanged("Day");
            }
        }
 
        private string _infos;
        public string Infos
        {
            get { return _infos; }
            set
            {
                _infos = value;
                OnPropertyChanged("Infos");
            }
        }
 
        private void OnPropertyChanged(string propertyName)
        {
            if(PropertyChanged!=null)
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
 
        #region INotifyPropertyChanged Members
 
        public event PropertyChangedEventHandler PropertyChanged;
 
        #endregion
    }
 
    public static class DaysConverter
    {
        private static readonly List<string> DaysStrings = new List<string> { "Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche" };
 
        public static string GetDayString(int day) 
        {
            return DaysStrings[day]; 
        }
 
        public static int GetDayInt(string day)
        {
            return DaysStrings.IndexOf(day);
        }
    }
 
    public class DeliveryDayView : INotifyPropertyChanged
    {
 
        public DeliveryDayView(DeliveryDayModel model, string day)
        {
            Day = day;
            Model = model;
            IsDeliveryPossible = true;
        }
 
        private DeliveryDayModel _model;
        public DeliveryDayModel Model
        {
            get { return _model; }
            set
            {
                _model = value;
                Model.PropertyChanged += new PropertyChangedEventHandler(Model_PropertyChanged);
            }
        }
 
        void Model_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            switch (e.PropertyName)
            {
                case "Day":
                    Day = DaysConverter.GetDayString(Model.Day);
                    break;
                case "Infos":
                    Infos = Model.Infos;
                    break;
            }
        }
 
        private string _day;
        public string Day
        {
            get { return _day; }
            set
            {
                _day = value;
                OnPropertyChanged("Day");
            }
        }
 
        private string _infos;
        public string Infos
        {
            get { return _infos; }
            set
            {
                _infos = value;
                OnPropertyChanged("Infos");
            }
        }
 
        private bool _IsDeliveryPossible;
        public bool IsDeliveryPossible
        {
            get { return _IsDeliveryPossible; }
            set
            {
                _IsDeliveryPossible = value;
                OnPropertyChanged("IsDeliveryPossible");
            }
        }
 
        private void OnPropertyChanged(string propertyName)
        {
            if(PropertyChanged!=null)
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
 
            if (propertyName == "IsDeliveryPossible" && IsDeliveryPossibleChanged != null)
                IsDeliveryPossibleChanged(this, null);
            else if(Model != null)
            {
                switch (propertyName)
                {
                    case "Day":
                        Model.Day = DaysConverter.GetDayInt(Day);
                        break;
                    case "Infos":
                        Model.Infos = Infos;
                        break;
                }       
            }
        }
 
        public event EventHandler IsDeliveryPossibleChanged;
 
        #region INotifyPropertyChanged Members
 
        public event PropertyChangedEventHandler PropertyChanged;
 
        #endregion    
    }

En fait tous les DeliveryDayView n'ont pas forcément de modèle correspondant (si leur propriété IsDeliveryPossible est à false). Donc ShippingController a un peu plus de travail que prévu car il doit instancier un nouveau DeliveryDayModel si la propriété IsDeliveryPossible est passée à true. A moins qu'il aurait mieux fallu systématiquement créer un DeliveryDayModel quand on crée un DeliveryDayView ?
De plus, la méthode ToogleDeliveryDay() du contrôleur reçoit à présent en paramètre un DeliveryDayView. C'est donc dans cette méthode que doit se faire le test "dayView.IsDeliveryPossible", il ne plus se faire dans ShippingModel.
Enfin, la synchronisation DeliveryDayModel<->DeliveryDayView est assuré par DeliveryDayView puisqu'il n'y a pas de contrôleur.

Le code devient quand même nettement plus lourd que l'original...

As-tu des remarques à faire dessus ?

[Maniak]

A moins qu'il aurait mieux fallu systématiquement créer un DeliveryDayModel quand on crée un DeliveryDayView ? :roll:

Pas nécessaire. En revanche :

Enfin, la synchronisation DeliveryDayModel<->DeliveryDayView est assuré par DeliveryDayView puisqu'il n'y a pas de contrôleur.

Là il y a une petite remarque :)
Actuellement, la gestion de DeliveryDayModel est séparée entre DeliveryDayView dans un cas et le contrôleur dans l'autre. Il y a moyen d'améliorer ça pour que le contrôleur contrôle tout.

Déjà extraire la partie de ToogleDeliveryDay qui obtient un DeliveryDayModel dans une méthode dédiée à 'convertir' un DeliveryDayView en DeliveryDayModel (ça de toute façon, ce serait mieux :)

Ensuite, un DeliveryDayView n'a plus besoin de son modèle une fois créé. Tu en copies déjà les valeurs. Donc si tu l'oublies, tu peux simplifier le code de DeliveryDayView en retirant toute la partie de synchro du modèle dans le cas où il y en a un.

Et côté contrôleur, tu demandes au modèle de te retrouver le DeliveryDayModel. S'il y arrive, tu le retires, sinon tu fais l'ajout.

Le code devient quand même nettement plus lourd que l'original...

Ça jongle entre plusieurs classes, mais au moins chacune a un job séparé :)

As-tu des remarques à faire dessus ?

Quelques-unes :)

Basé sur ce code-ci, il y a plusieurs choses qui n'ont pas l'air utiles. Mais ça peut être utilisé par ailleurs, donc c'est juste au cas-où.

ShippingModel :
- si personne n'appelle ShippingModel.set_Days. Pas de raison que ça existe. Un setter sur une collection, ça devrait lever un flag d'alerte.

- l'évènement PropertyChanged devrait être déclenché dans AddDeliveryDay et RemoveDeliveryDay vu que ce sont ces méthodes qui modifient la collection.

DeliveryDayModel :
- le Model dans le nom est superflu. c'est une classe qui représente un jour de livraison, DeliveryDay tout court lui va très bien. Ça fait partie du langage du domaine.

- les évènements sur modification d'une propriété de DeliveryDay ne sont pas utilisés ici. Même si le modèle était modifié par ailleurs, à moins qu'une des propriétés de DeliveryDay soit très importante à mettre à jour instantanément à l'affichage (Day ne devrait pas changer, Infos je sais pas :), tu peux zapper tout ça. En particulier si toute la collection de DeliveryDayView est recréée quand il y a une modification sur la collection de DeliveryDay du modèle, ça peut être bien suffisant comme synchro.

- si DeliveryDayView.Day n'est pas modifiable, le setter peut virer. Idem pour Infos.

DeliveryDayView :
- DeliveryDayView.set_IsDeliveryPossible peut lancer l'évènement IsDeliveryPossibleChanged directement, plutôt que d'avoir besoin que OnPropertyChanged devine d'où elle est appelée.

- si IsDeliveryPossibleChanged est le seul évènement utilisé pour déclencher un traitement via le contrôleur, tu peux virer OnPropertyChanged directement.

- ShippingController.View n'a pas besoin d'être une propriété publique, juste un champ privé.


Au final, selon ce qui est réellement utilisé, tu pourrais potentiellement virer :
- ShippingModel.set_Days
- DeliveryDay.set_Day
- DeliveryDay.OnPropertyChanged
- DeliveryDay.PropertyChanged
- DeliveryDayView.Model
- DeliveryDayView.Model_PropertyChanged
- DeliveryDayView.set_Day
- DeliveryDayView.set_Infos
- DeliveryDayView.OnPropertyChanged
- DeliveryDayView.PropertyChanged

Enfin, pourquoi est-ce que tu déclares les champs privés à côté des propriétés qui les exposent ? Mis à part que la convention veut qu'ils soient tous déclarés en début de classe, l'existence de propriétés n'est pas systématique, loin de là. On ne devrait créer une propriété que si on a *vraiment* besoin d'exposer les données internes, s'il n'y a vraiment pas d'alternative. Et les setters devraient être bien plus rares que les getters. Encapsulation oblige, une classe ne doit pas exposer ses détails au reste du monde, surtout en modification, si ce n'est pas absolument nécessaire.
Ici tu as des get/set systématiques, mais est-ce qu'ils sont vraiment utilisés ? Est-ce que n'importe qui est censé pouvoir remplacer la collection Days du modèle ? Le modèle d'un DeliveryDayView ?

Mais bon, là on n'est plus vraiment dans le cadre du MVC :)

[maa]

un DeliveryDayView n'a plus besoin de son modèle une fois créé. Tu en copies déjà les valeurs. Donc si tu l'oublies, tu peux simplifier le code de DeliveryDayView en retirant toute la partie de synchro du modèle dans le cas où il y en a un.

Et côté contrôleur, tu demandes au modèle de te retrouver le DeliveryDayModel. S'il y arrive, tu le retires, sinon tu fais l'ajout.

Tu veux dire qu'un DeliveryDayView ne devrait pas contenir une référence à DeliveryDayModel ? Dans ce cas il faut que le contrôleur le retrouve en parcourant la collection model.Days, c'est un peu moins performant...

Ca donnerait donc un code comme ça dans le contrôleur:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            DeliveryDay dayModel = GetModel(dayView);
            if(dayView.IsDeliveryPossible)
                model.AddDeliveryDay(dayModel);
            else
                model.RemoveDeliveryDay(dayModel);
        }
 
        public DeliveryDay GetModel(DeliveryDayView view)
        {
            int day = DaysConverter.GetDayInt(view.Day);
            return model.Days.FirstOrDefault(y => y.Day == day) ??
                   new DeliveryDay(DaysConverter.GetDayInt(view.Day));
        }

Maintenant il faudrait que le modèle soit modifié quand on set les propriétés de la vue. Il faudrait donc le faire par événement et ça serait encore ShippingController qui s'en chargerait ?

si personne n'appelle ShippingModel.set_Days. Pas de raison que ça existe. Un setter sur une collection, ça devrait lever un flag d'alerte.

En fait, la collection Days n'est settée qu'au chargement des données par l'ORM. C'est à ce moment qu'on doit créer en parallèle la collection AvailableDays.

- l'évènement PropertyChanged devrait être déclenché dans AddDeliveryDay et RemoveDeliveryDay vu que ce sont ces méthodes qui modifient la collection.

En fait il n'y a que le contrôleur qui est censé appeler ces méthodes. Si on lève un événement, ShippingView va recréer une collection de DeliveryDayView et ça à chaque fois qu'une propriété DeliveryDayView.IsDeliveryPossible est settée...
De plus rien n'empêche aussi d'ajouter ou supprimer des DeliveryDayModel en invoquant Days.Add() ou Days.Remove()...

- les évènements sur modification d'une propriété de DeliveryDay ne sont pas utilisés ici. Même si le modèle était modifié par ailleurs, à moins qu'une des propriétés de DeliveryDay soit très importante à mettre à jour instantanément à l'affichage (Day ne devrait pas changer, Infos je sais pas , tu peux zapper tout ça. En particulier si toute la collection de DeliveryDayView est recréée quand il y a une modification sur la collection de DeliveryDay du modèle, ça peut être bien suffisant comme synchro.

En fait les propriétés des modèles sont setté au chargement des données puis par la suite sont modifiées seulement quand la vue change. Mais il pourrait arriver plus tard que les modèles soient modifiés directement par le programme et dans ce cas il faudrait notifier la vue. D'ailleurs à ce sujet, si la vue fait model.Infos = "blabla", le modèle renvois un événement pour dire que Infos a été modifié et la vue se remet à jour. Ca fait un retour un peu inutile puisque la modification vient de la vue.

- si DeliveryDayView.Day n'est pas modifiable, le setter peut virer. Idem pour Infos.
- si IsDeliveryPossibleChanged est le seul évènement utilisé pour déclencher un traitement via le contrôleur, tu peux virer OnPropertyChanged directement.

oui

- ShippingController.View n'a pas besoin d'être une propriété publique, juste un champ privé.

Il faut pourtant bien demander au contrôleur de fournir la vue pour binder celle-ci à un contrôle de l'interface, non ?

Autre question : Est-ce que la vue, en MVC peut être directement un contrôle de l'UI (je pense par exemple à un usercontrol) ? (Je veux dire que dans ce cas il n'y aurait pas de ViewModel).

Merci pour tous ces conseils.

[Maniak]

Tu veux dire qu'un DeliveryDayView ne devrait pas contenir une référence à DeliveryDayModel ? Dans ce cas il faut que le contrôleur le retrouve en parcourant la collection model.Days, c'est un peu moins performant...

Ouaip, mais ça centralise le traitement à un seul endroit. Alors vu qu'il y a très peu de chances que le ralentissement se sente et que les préoccupations de performance passent après les préoccupations de design... :)

Ca donnerait donc un code comme ça dans le contrôleur:

Quasiment. Le code dans GetModel devrait être sur le modèle. Même principe que pour les ajouts/suppressions, le contrôleur n'a pas à savoir comment sont implémentés les jours dans le modèle et comment y farfouiller. Donc je verrais plutôt du genre

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            DeliveryDay dayModel = model.GetModel(dayView.Day);
            if(dayModel == null)
                model.AddDeliveryDay(dayView.Day);
            else
                model.RemoveDeliveryDay(dayModel);
        }

Enfin pour pinailler, ça pourrait être

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            string day = dayView.Day;
            if (model.HasDeliveryDay(day))
                model.RemoveDeliveryDay(day);
            else
                model.AddDeliveryDay(day);
        }

Pas de null, pas besoin de savoir comment trouver, créer, ajouter ou retirer un DeliveryDay, la méthode ToggleDeliveryDay fait exactement ce que son nom indique, toujours à un même niveau d'abstraction.

Et pour te faire protester, ça pourrait aussi être :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            model.ToggleDeliveryDay(dayView.Day);
        }

:)

Maintenant il faudrait que le modèle soit modifié quand on set les propriétés de la vue. Il faudrait donc le faire par événement et ça serait encore ShippingController qui s'en chargerait ?

Yup.

En fait, la collection Days n'est settée qu'au chargement des données par l'ORM.

Il y aurait moyen de la passer dans le constructeur du modèle ? Ou au pire d'en faire une méthode avec un nom plus explicite, du genre InitializeDays ?

C'est à ce moment qu'on doit créer en parallèle la collection AvailableDays.

AvailableDays c'est de l'affichage, ça ne se situe pas au même niveau, donc leurs initialisations ne devraient pas être liées. Rien n'oblige la vue d'être liée au modèle au moment ou il est initialisé.

En fait il n'y a que le contrôleur qui est censé appeler ces méthodes. Si on lève un événement, ShippingView va recréer une collection de DeliveryDayView et ça à chaque fois qu'une propriété DeliveryDayView.IsDeliveryPossible est settée...

Ben il faut bien que la vue soit au courant quand un jour est ajouté/supprimé. Là elle n'est prévenue que quand toute la collection Days est remplacée, ce qui n'arrive donc qu'à l'initialisation. Il y a quelque chose qui cloche :)
Et tu peux toujours faire en sorte que la vue ne recrée pas l'intégralité de la collection à chaque fois, mais fasse juste le delta.

De plus rien n'empêche aussi d'ajouter ou supprimer des DeliveryDayModel en invoquant Days.Add() ou Days.Remove()...

Si, toi :)
Modifier directement les collections d'autres objets, c'est pas terrible terrible niveau encapsulation. Obliger à passer par des méthodes du genre AddDay et RemoveDay sur l'objet contenant est plus sain, notamment parce que ça permet de t'assurer que les règles métier sont appliquées. Si tu as besoin de faire des vérifications particulières quand tu ajoutes ou retires un jour, comment est-ce que tu fais si tu permets à n'importe qui de zapper le modèle et d'aller taper directement dans la collection ?

Cela dit, on n'est pas vraiment aidés à enforcer ça avec .NET vu qu'il n'y a pas d'interface 'lecture seule' pour les classes de collection. Il faut compter sur les développeurs et les tests unitaires (ou faire un peu de plomberie). Le plus proche, c'est de renvoyer une ReadOnlyCollection<T> dans le getter, qui lance une exception sur les méthodes de modification. C'est pas la solution la plus légère.

En fait les propriétés des modèles sont setté au chargement des données puis par la suite sont modifiées seulement quand la vue change.

Même chose que pour le setter de Days donc, est-ce qu'il y aurait moyen de faire l'initialisation soit directement dans le constructeur, soit via une ou des méthodes InitializeXxx dont le nom indique clairement que ce n'est utilisé que pour l'initialisation ?

Mais il pourrait arriver plus tard que les modèles soient modifiés directement par le programme et dans ce cas il faudrait notifier la vue. D'ailleurs à ce sujet, si la vue fait model.Infos = "blabla", le modèle renvois un événement pour dire que Infos a été modifié et la vue se remet à jour. Ca fait un retour un peu inutile puisque la modification vient de la vue.

Elle peut toujours regarder si la valeur a changé avant de se mettre à jour. Et pour ce qui est du déclenchement d'évènement superflu... bah *quand* tu auras besoin de permettre la modification de model.Infos par autre chose que la vue, *si* ça pose un problème, *alors* il faudra aviser. Il y a probablement un peu de marge avant que ça arrive :)

Dans l'immédiat, si tu n'as pas besoin de propager les modifs sur une propriété, pas besoin de faire la plomberie correspondante (YAGNI).

Il faut pourtant bien demander au contrôleur de fournir la vue pour binder celle-ci à un contrôle de l'interface, non ?

Ben la vue gérée par le contrôleur *est* l'interface en principe :)

Autre question : Est-ce que la vue, en MVC peut être directement un contrôle de l'UI (je pense par exemple à un usercontrol) ? (Je veux dire que dans ce cas il n'y aurait pas de ViewModel).

Oui donc, c'est même censé être ça à la base :)
Le ViewModel c'est essentiellement pour ajouter une couche entre l'interface graphique avec tous ses contrôles (la vue donc) et le contrôleur/presenter. Cf Presentation Model.
C'est notamment ce qui permet d'isoler le contrôleur/presenter de la librairie graphique utilisée.

[maa]

Il y aurait moyen de la passer dans le constructeur du modèle ? Ou au pire d'en faire une méthode avec un nom plus explicite, du genre InitializeDays ?

Non, nhibernate à besoin d'un constructeur par défaut pour charger les objets. Par contre je crois que je peux rendre mes setter privés.

Pour la propriété Infos, elle a besoin d'être mise à jour quand la propriété correspondante de la vue est mise à jour. Donc elle doit être public. Il faudrait que le contrôleur seulement puisse la setter, sinon elle ne sera plus synchronisée avec la vue car j'ai supprimé la plomberie événementielle.

Ben il faut bien que la vue soit au courant quand un jour est ajouté/supprimé. Là elle n'est prévenue que quand toute la collection Days est remplacée, ce qui n'arrive donc qu'à l'initialisation. Il y a quelque chose qui cloche

A l'origine c'est la vue qui demande d'ajouter ou de supprimer un jour dans la collection du model, donc elle n'a pas besoin d'être ré informée. La propriété IsDeliveryPossible du DeliveryDayView correspondant est déjà à jour.

Ben la vue gérée par le contrôleur *est* l'interface en principe

Dans mon cas, la vue n'est pas directement l'interface graphique (c'est un viewModel si j'ai bien compris). Est-ce toujours du MVC ?
Toujours est-il qu'il faut bien que quelqu'un fournisse la vue. N'est ce pas le rôle du contrôleur ?

Allez voici une nouvelle version du code (espérons quasiment parfaite cette fois ci)

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
 
   public class Shipping
    {
        public IList<DeliveryDay> Days { get; private set; }
 
        public void ToogleDeliveryDay(int day)
        {
            var deliveryday = Days.FirstOrDefault(y => y.Day == day);
            if(deliveryday == null)
                Days.Add(GetDeliveryDay(day));
            else
                Days.Remove(deliveryday);
        }
 
        public DeliveryDay GetDeliveryDay(int day)
        {
            return Days.FirstOrDefault(y => y.Day == day) ??
                   new DeliveryDay(day);
        }
    }
 
    public class ShippingView
    {
        private Shipping model;
 
        private ShippingController controller;
 
        public ShippingView(Shipping model, ShippingController controller)
        {
            this.model = model;
            this.controller = controller;
            AvailableDays = new ObservableCollection<DeliveryDayView>();
            for (int i = 0; i < 7; i++)
            {
                var dayView = new DeliveryDayView(DaysConverter.GetDayString(i));
                dayView.PropertyChanged += new PropertyChangedEventHandler(day_PropertyChanged);
                dayView.IsDeliveryPossibleChanged += new EventHandler(day_IsDeliveryPossibleChanged);
                AvailableDays.Add(dayView);
            }
        }
 
        private void day_IsDeliveryPossibleChanged(object sender, EventArgs e)
        {
            controller.ToogleDeliveryDay(sender as DeliveryDayView);
        }
 
        private void day_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            controller.UpdateModel(sender as DeliveryDayView);
        }
 
        public ObservableCollection<DeliveryDayView> AvailableDays { get; set; }
    }
 
    public class ShippingController
    {
        private Shipping model;
        private ShippingView view;
 
        public ShippingController(Shipping model)
        {
            this.model = model;
            view = new ShippingView(model, this);
        }
 
        public void ToogleDeliveryDay(DeliveryDayView dayView)
        {
            model.ToogleDeliveryDay(DaysConverter.GetDayInt(dayView.Day));
        }
 
        public void UpdateModel(DeliveryDayView dayView)
        {
            var dayModel = model.GetDeliveryDay(DaysConverter.GetDayInt(dayView.Day));
            dayModel.Infos = dayView.Infos;
        }
    }
 
    public class DeliveryDay
    {
        public DeliveryDay(int day)
        {
            Day = day;
        }
 
        private DeliveryDay() {}
 
        public int Day { get; private set; }
 
        public string Infos { get; set; }
    }
 
    public static class DaysConverter
    {
        private static readonly List<string> DaysStrings = new List<string> { "Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", "Samedi", "Dimanche" };
 
        public static string GetDayString(int day) 
        {
            return DaysStrings[day]; 
        }
 
        public static int GetDayInt(string day)
        {
            return DaysStrings.IndexOf(day);
        }
    }
 
    public class DeliveryDayView : INotifyPropertyChanged
    {
 
        public DeliveryDayView(string day)
        {
            Day = day;
            IsDeliveryPossible = true;
        }
 
        public string Day { get; private set; }
 
        private string _infos;
        public string Infos
        {
            get { return _infos; }
            set
            {
                _infos = value;
                OnPropertyChanged("Infos");
            }
        }
 
        private bool _IsDeliveryPossible;
        public bool IsDeliveryPossible
        {
            get { return _IsDeliveryPossible; }
            set
            {
                _IsDeliveryPossible = value;
                if(IsDeliveryPossibleChanged != null)
                    IsDeliveryPossibleChanged(this, null);
            }
        }
 
        private void OnPropertyChanged(string propertyName)
        {
            if(PropertyChanged!=null)
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
 
        public event EventHandler IsDeliveryPossibleChanged;
 
        #region INotifyPropertyChanged Members
 
        public event PropertyChangedEventHandler PropertyChanged;
 
        #endregion    
    }

Encore des amélioration possibles ?

[Maniak]

Non, nhibernate à besoin d'un constructeur par défaut pour charger les objets. Par contre je crois que je peux rendre mes setter privés.

protected virtual si tu veux que ce soit lazy-loadable :)

A l'origine c'est la vue qui demande d'ajouter ou de supprimer un jour dans la collection du model, donc elle n'a pas besoin d'être ré informée. La propriété IsDeliveryPossible du DeliveryDayView correspondant est déjà à jour.

Ok, donc pareil que pour le reste, si la vue est la seule source de modification de ces infos, pas besoin de remonter les évènements de modif depuis le modèle. Mais si une autre action sur un autre élément de l'interface se traduisait par des ajouts/suppressions de jours (un bouton Annuler pour revenir à l'état initial par exemple), il faudra bien que la vue soit mise au courant qu'il faut remettre à jour la liste.

Dans mon cas, la vue n'est pas directement l'interface graphique (c'est un viewModel si j'ai bien compris). Est-ce toujours du MVC ?

C'est... discutable :)
Où est la 'vraie' vue dans tout ça, avec les vrais contrôles ? À quel endroit fais-tu la liaison entre la collection de DeliveryDayView et le contrôle correspondant ?
Toute la plomberie évenementielle est censée être dans la vue, avec les contrôles.

Toujours est-il qu'il faut bien que quelqu'un fournisse la vue. N'est ce pas le rôle du contrôleur ?

À mon sens c'est plutôt dans l'autre sens :)
La vue sait à quel contrôleur elle doit être rattachée. Le contrôleur ne sait pas forcément quelle(s) vue(s) l'utilise(nt).

Avec MonoRail et ASP.NET MVC, le framework part de l'uri utilisée pour déterminer la vue (.vm, .aspx, ...) et son contrôleur. C'est lui qui fournit les deux. Il appelle ensuite le contrôleur pour effectuer l'action en cours (aussi depuis l'uri). On peut assimiler cette partie-là au comportement de la vue.

En WinForms, WebForms et ASP.NET 'brut', le point d'entrée est l'interface. Tu affiches un formulaire, tu demandes un .aspx, c'est de là que tout part. Donc à défaut d'obtenir le couple vue/contrôleur automatiquement comme avec ASP.NET MVC, le formulaire peut créer le contrôleur et le modèle dont il a besoin.

Sinon il faut monter un cran au-dessus, là où le formulaire à charger est déterminé, pour pouvoir tout créer à la fois.

Allez voici une nouvelle version du code (espérons quasiment parfaite cette fois ci;))

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

...

Encore des amélioration possibles ?

Mmh, UpdateModel n'est pas très explicite comme nom, UpdateDeliveryDay serait plus clair (sans avoir besoin de voir le type du paramètre).
Sinon, mis à part la relation entre vue et contrôleur que je verrais plutôt dans l'autre sens donc, et le fait qu'il manque la /vraie/ vue dans le paysage, ça semble aller dans le bon sens :)

[maa]

À mon sens c'est plutôt dans l'autre sens
La vue sait à quel contrôleur elle doit être rattachée. Le contrôleur ne sait pas forcément quelle(s) vue(s) l'utilise(nt).

Je disais que le contrôleur devait afficher la vue parce que c'est ce que j'ai lu dans le tutoriel de mon premier post :

On va maintenant créer la classe "main" de l'application. Sa fonction est plus que simple, elle crée un nouveau modèle, crée un nouveau contrôleur en lui passant le modèle et demande au contrôleur d'afficher les vues.

Voici donc à quoi elle va ressembler :
JVolume.java

public class JVolume {
public static void main(String[] args) {
VolumeModel model = new VolumeModel(50);
VolumeController controller = new VolumeController(model);
controller.displayViews();
}
}

Faut-il procéder différemment dans mon cas ?

Où est la 'vraie' vue dans tout ça, avec les vrais contrôles ? À quel endroit fais-tu la liaison entre la collection de DeliveryDayView et le contrôle correspondant ?

Alors je dirais que ma vrai vue est "window1.xaml" (avec une datagrid pour afficher les shipping et une pour afficher les deliveryday) et dans le code behind j'écris:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
            var shippings = _session.GetNamedQuery("shipping_query").List<Shipping>();
            var shippingControllers = from m in shippings
                                      select new ShippingController(m);
            var shippingViews = (from c in shippingControllers
                                select c.ShippingView).ToList();
 
            DataContext = shippingViews;

et tout le reste s'affiche grâce au binding dans le XAML.

[Maniak]

Je disais que le contrôleur devait afficher la vue parce que c'est ce que j'ai lu dans le tutoriel de mon premier post :

Faut-il procéder différemment dans mon cas ?

Bah ça veut dire que pour cette partie-là, le MVC décrit dans ce tutoriel ne correspond pas à la définition qui en est faite par Fowler. Et perso, vu que c'est la sienne que je prends comme référence... :)

Alors je dirais que ma vrai vue est "window1.xaml" (avec une datagrid pour afficher les shipping et une pour afficher les deliveryday) et dans le code behind j'écris:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

...

et tout le reste s'affiche grâce au binding dans le XAML.

Mmh... okok. D'un côté ça éclaircit bien les choses, de l'autre ça veut dire qu'il y a des embrouilles :)

J'en ai perdu le fil du coup, mais après un tour dans le code, ça semble s'éclaircir.

Donc :)

Il y avait déjà confusion sur ce à quoi correspond la vue, ça on l'a... euh... vu :)
Du coup il y a confusion sur l'endroit où se trouve *le* contrôleur.
Et en cadeau bonus, il y a aussi confusion sur ce à quoi correspond le modèle.

Au moins ça va, ça couvre l'ensemble du triplet MVC :)

Après le petit tour dans le code pour me remettre les idées au clair, voilà où j'en suis arrivé. Tu me dis si ça te parle :)

1. Le modèle
3 classes : DeliveryDay, Shipping et... ShippingSchedulerModel (nom totalement improvisé :)
Pour DeliveryDay, pas de changements particuliers (juste lazy-loading-friendly pour NHibernate :) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
	public class DeliveryDay
	{
		public DeliveryDay(int day, string infos)
		{
			Day = day;
			Infos = infos;
		}
 
		protected DeliveryDay()
		{
		}
 
		public virtual int Day { get; protected set; }
 
		public virtual string Infos { get; set; }
	}

Pour Shipping non plus, si ce n'est que le Toggle m'embêtait un peu en fait. Donc là, Shipping représente une livraison, avec sa collection de jours et de quoi retrouver, ajouter et retirer un jour (il manque les autres infos dont tu as besoin évidemment) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
	public class Shipping
	{
		public virtual IList<DeliveryDay> Days { get; protected set; }
 
		public virtual void AddDeliveryDay(int dayIndex, string infos)
		{
			Days.Add(new DeliveryDay(dayIndex, infos));
		}
 
		public virtual void RemoveDeliveryDay(DeliveryDay deliveryDay)
		{
			Days.Remove(deliveryDay);
		}
 
		public virtual DeliveryDay FindDeliveryDay(int dayIndex)
		{
			return Days.First(x => x.Day == dayIndex);
		}
	}

ShippingSchedulerModel maintenant, un des chaînons manquants. C'est lui qui discute avec la couche d'accès aux données pour récupérer les objets du domaine (DeliveryDay et Shipping ici) et effectuer les traitements métier (aucun ici :) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
	public class ShippingSchedulerModel
	{
		public IList<Shipping> FindShippings()
		{
			return new List<Shipping>();
		}
 
		public void SaveChanges()
		{
		}
	}

Tel quel, il est particulièrement passionnant, mais évidemment il faut lui ajouter la liaison avec la couche d'accès aux données pour qu'il serve à quelque chose (c'est là qu'on sort l'injection de dépendance). SaveChanges, c'est là où tu pourrais faire un transaction.Commit(). C'est le contrôleur qui décidera de quand le faire.

2. La vue
5 classes : DaysConverter, DeliveryDayView, ShippingView, ShippingViewEventArgs et... ShippingSchedulerForm.

DaysConverter, j'ai juste joué avec les noms parce que mon pseudo n'est pas un hasard :)

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
	public static class DaysConverter
	{
		private static readonly string[] _dayNames = {
		                                             	"Lundi", "Mardi", "Mercredi", "Jeudi", "Vendredi", 
		                                             	"Samedi", "Dimanche"
		                                             };
 
		public static string GetDayName(int index)
		{
			return _dayNames[index];
		}
 
		public static int GetDayIndex(string name)
		{
			return Array.IndexOf(_dayNames, name);
		}
	}

DeliveryDayView est globalement toujours pareil, si ce n'est qu'il a 2 constructeurs, un à partir d'un DeliveryDay et un à partir d'un numéro de jour. Aussi les deux setters sur Infos et IsDeliveryPossible déclenchent le même évènement. La distinction ne semblait pas vitale (mais elle peut l'être, auquel cas il suffit de la remettre).

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
	public class DeliveryDayView : INotifyPropertyChanged
	{
		private string _infos;
		private bool _isDeliveryPossible;
 
		public DeliveryDayView(DeliveryDay day)
		{
			Day = DaysConverter.GetDayName(day.Day);
			Infos = day.Infos;
			IsDeliveryPossible = true;
		}
 
		public DeliveryDayView(int dayIndex)
		{
			Day = DaysConverter.GetDayName(dayIndex);
			Infos = string.Empty;
		}
 
		public event PropertyChangedEventHandler PropertyChanged = (sender, e) => { };
 
		public string Day { get; private set; }
 
		public string Infos
		{
			get { return _infos; }
			set
			{
				_infos = value;
				PropertyChanged(this, new PropertyChangedEventArgs("Infos"));
			}
		}
 
		public bool IsDeliveryPossible
		{
			get { return _isDeliveryPossible; }
			set
			{
				_isDeliveryPossible = value;
				PropertyChanged(this, new PropertyChangedEventArgs("IsDeliveryPossible"));
			}
		}
	}

ShippingView, ce que tu traitais comme le V de MVC alors que ce n'est qu'un élément de présentation au même titre que DeliveryDayView, lui bouge un peu. Il référence toujours un Shipping, il n'a plus de contrôleur, il s'attache à l'évènement de modification des différents DeliveryDayView et le rebalance dans un évènement à lui.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
	public class ShippingView
	{
		public ShippingView(Shipping shipping)
		{
			Shipping = shipping;
			AvailableDays = new DeliveryDayView[7];
 
			RefreshAvailableDays();
		}
 
		public event EventHandler<ShippingViewEventArgs> DeliveryDayChanged = (sender, e) => { }
 
		public Shipping Shipping { get; private set; }
 
		public DeliveryDayView[] AvailableDays { get; set; }
 
		private void OnDeliveryDayPropertyChanged(object sender, PropertyChangedEventArgs e)
		{
			DeliveryDayChanged(this, new ShippingViewEventArgs(this, sender as DeliveryDayView));
		}
 
		private void RefreshAvailableDays()
		{
			for (int i = 0; i < 7; ++i)
			{
				var deliveryDay = Shipping.FindDeliveryDay(i);
 
				DeliveryDayView view;
				if (deliveryDay != null)
				{
					view = new DeliveryDayView(deliveryDay);
				}
				else
				{
					view = new DeliveryDayView(i);
				}
 
				AvailableDays[i] = view;
				view.PropertyChanged += OnDeliveryDayPropertyChanged;
			}
		}
	}

ShippingViewEventArgs, pas la peine de le mentionner :)

ShippingSchedulerForm enfin, l'autre chaînon manquant, ton codebehind et le vrai V de ton MVC. Il référence son modèle et son contrôleur (l'injection de dépendances est bienvenue pour ça aussi). Au chargement il récupère la collection de Shipping depuis le modèle, il en fait une collection de ShippingView en s'attachant au passage à l'évènement de mise à jour des DeliveryDayViews, et il binde là-dessus.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
	public class ShippingSchedulerForm
	{
		private readonly ShippingSchedulerModel _model;
		private readonly ShippingSchedulerController _controller;
 
		public ShippingSchedulerForm()
		{
			_model = new ShippingSchedulerModel();
			_controller = new ShippingSchedulerController(_model);
		}
 
		public void Load()
		{
			IList<Shipping> shippings = _model.FindShippings();
 
			var shippingViews = new List<ShippingView>();
			foreach (var shipping in shippings)
			{
				var shippingView = new ShippingView(shipping);
				shippingView.DeliveryDayChanged += OnShippingViewDeliveryDayChanged;
 
				shippingViews.Add(shippingView);
			}
 
			DataContext = shippingViews;
		}
 
		private void OnShippingViewDeliveryDayChanged(object sender, ShippingViewEventArgs e)
		{
			_controller.UpdateDeliveryDay(e.ShippingView, e.DeliveryDayView);
		}
	}

Sur le principe ce n'est pas très éloigné de ce que tu as fait, si ce n'est que l'accès aux données se fait dans le modèle et qu'il n'y a pas de contrôleur pour ShippingView.


3. Le contrôleur
Plus simple, 1 classe : ShippingSchedulerController.
Il ne gère qu'une seule action pour l'instant, à savoir la mise à jour d'un DeliveryDay.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
	public class ShippingSchedulerController
	{
		private readonly ShippingSchedulerModel _model;
 
		public ShippingSchedulerController(ShippingSchedulerModel model)
		{
			_model = model;
		}
 
		public void UpdateDeliveryDay(ShippingView shippingView, DeliveryDayView deliveryDayView)
		{
			var shipping = shippingView.Shipping;
 
			int dayIndex = DaysConverter.GetDayIndex(deliveryDayView.Day);
			var deliveryDay = shipping.FindDeliveryDay(dayIndex);
 
			if (deliveryDay != null)
			{
				if (!deliveryDayView.IsDeliveryPossible)
				{
					shipping.RemoveDeliveryDay(deliveryDay);
				}
				else
				{
					deliveryDay.Infos = deliveryDayView.Infos;
				}
			}
			else if (deliveryDayView.IsDeliveryPossible)
			{
				shipping.AddDeliveryDay(dayIndex, deliveryDayView.Infos);
			}
 
			_model.SaveChanges();
		}
	}

Vu que les modifications sur Infos et IsDeliveryPossible lèvent le même évènement, le traitement combine les deux (ça peut être séparé, mais j'avais pas le courage là :). À la fin du traitement, le contrôleur indique au modèle qu'il peut persister les changements dans la base.


Et... voilà.

Dans le sens vue -> modèle, il manque un évènement quand les propriétés (manquantes) sur ShippingView sont modifiées, évènement auquel s'attacherait la vue qui appellerait ensuite le contrôleur pour lui dire de mettre à jour le shipping concerné (un OnShippingViewChanged quoi).

Dans le sens modèle -> vue, vu qu'il n'y a plus d'évènement suite à une mise à jour dans les propriétés de DeliveryDay et Shipping, tout se fait toujours à sens unique, mais ce ne serait pas bien difficile de le remettre.
Comme pour DeliveryDayView et ShippingView, les évènements de mise à jour sur DeliveryDay seraient interceptés par Shipping et rebalancés dans un autre évènement. Ton codebehind s'attacherait à cet évènement au chargement et rafraîchirait ou recréerait la collection de shippingViews quand il se déclenche.


Bon bah il aura encore été super long ce post. Mais avec un peu de chance ce sera plus clair :)

[maa]

Merci beaucoup pour cette réponse très complète. C'est beaucoup plus clair maintenant pour moi.

En fait je faisais le chargement des données dans la vue, en me rendant compte que ça n'était pas super. Là c'est nettement plus propre.

Un seul point que je n'ai pas bien saisi est quand tu parles d'injection de dépandance Pour ShippingSchedulerModel et pour le contrôleur. Je n'ai jamais trop utilisé ce principe (où peut être sans le savoir ). Je sais qu'il s'agit d'utiliser des interfaces pour diminuer le couplage entre les classes. Je suppose ici pour pouvoir changer facilement les class ShippingSchedulerModel et ShippingSchedulerController.
Pourrais-tu me donner un aperçu de ce que donnerait l'injection de dépendance dans mon exemple ?

Merci beaucoup pour ton aide.

[Maniak]

Un seul point que je n'ai pas bien saisi est quand tu parles d'injection de dépandance Pour ShippingSchedulerModel et pour le contrôleur. Je n'ai jamais trop utilisé ce principe (où peut être sans le savoir :roll:). Je sais qu'il s'agit d'utiliser des interfaces pour diminuer le couplage entre les classes.

Ce n'est pas directement lié. Le principe d'injection de dépendances est tout bête à la base. C'est juste qu'au lieu d'avoir une classe qui crée elle-même des instances d'une autre classe concrète, elle les obtient d'ailleurs, par exemple dans le constructeur ou via un setter.

Pour ta vue qui crée actuellement un ShippingSchedulerModel et un ShippingSchedulerController directement dans son constructeur, ça pourrait prendre la forme d'un constructeur avec 2 paramètres, auquel tu passerais modèle et contrôleur. Les dépendances sur modèle et contrôleur sont 'injectées' à la vue par la classe qui la crée.

Ça te donnerait un constructeur du genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
	public class ShippingSchedulerForm
	{
		private readonly ShippingSchedulerModel _model;
		private readonly ShippingSchedulerController _controller;
 
		public ShippingSchedulerForm(ShippingSchedulerModel model, ShippingSchedulerController controller)
		{
			_model = model;
			_controller = controller;
		}
	}

Pas obligatoirement d'interfaces donc. Tu remontes la responsabilité de créer le triplet MVC dans la classe qui crée la vue. À elle de savoir que le modèle a besoin de NHibernate, que le contrôleur a besoin du modèle et que la vue a besoin des deux.

Je suppose ici pour pouvoir changer facilement les class ShippingSchedulerModel et ShippingSchedulerController.

Quand tu passes par des interfaces, oui. Et c'est notamment *très* utile pour pouvoir faire des tests unitaires de la vue, du contrôleur et du modèle séparément, en leur injectant de 'fausses' dépendances dont tu contrôles le comportement.

Là c'est juste une des façons de faire. Tu aussi (entre autres) injecter les dépendances via un setter quand utiliser le constructeur n'est pas possible/souhaitable ou utiliser un framework comme Spring.NET.

À noter qu'injection de dépendances n'est pas synonyme d'inversion de contrôle. C'en est une forme, mais l'utilisation d'évènements pour piloter l'exécution d'une application est aussi de l'inversion de contrôle. Ce ne sont pas les classes qui décident quand une action doit se faire, ce sont des évènements extérieurs qui leur disent de se bouger un peu.

Pour plus de détails, toujours l'éternel Fowler ou Wikipedia (DI et IoC).