Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Design] Supprimer la dépendance cyclique de packages models <-> models.notifiers #108

Open
martialsomda opened this issue Apr 25, 2013 · 8 comments
Labels

Comments

@martialsomda
Copy link
Member

Dans l'état actuel, les objets Participant et Speaker utilisent la classe EmailNotification pour envoyer des email => Participant,Speaker dépend de EmailNotification
La classe EmailNotification prend en paramètre des objets Participant et Speaker => EmailNotification dépend de Participant,Speaker

Ce qui conduit à une dépendance cyclique. Il faut supprimer cette dépendance cyclique

@Centonni
Copy link
Member

L'envoi des mails lors de la modification des Speaker et Participant pourrait être déplacé vers les controllers, ce qui
ferait que les models ne dépendront plus de EmailNotification.
Ou carrément placer EmailNotification dans le même package que Speaker et Participant.
Qu'en pensez-vous?

@martialsomda
Copy link
Member Author

En fait c'est plus philosophique qu'autre chose..

Si on veut garder une approche model driven..l'envoie d'email fait partie intégrante du process de création du speaker ou du participant..et dans ce cas ça a du sens que la classe NotificationEmail soit utilisée par le model.
On pourrait juste supprimer la dépendance cyclique en faisant en sorte que les méthodes de la classe NotificationEmail prennent en paramètre des string par exemple.

Sinon ta solution est bonne aussi..moi j'aurai une préférence pour la solution que j'ai cité plus haut mais je m'en remet au jugement de la communauté :-)

Martial SOMDA

Le 29 août 2013 à 21:50, Komi Serge Innocent [email protected] a écrit :

L'envoi des mails lors de la modification des Speaker et Participant pourrait être déplacé vers les controllers, ce qui

ferait que les models ne dépendront plus de EmailNotification.
Ou carrément placer EmailNotification dans le même package que Speaker et Participant.
Qu'en pensez-vous?


Reply to this email directly or view it on GitHub.

@Centonni
Copy link
Member

j'ai bien pensé à ta solution et j'y adhère, mais j'étais un peu réticent à la proposer du fait que les différentes vues appelés, enroll.scala.html par exemple ,utilisent les model Member et Session.Il serait possible de trouver une solution pour ça.
Ne pourrait-on pas par exemple définir une couche d'abstraction pour les traitements métiers?je pense à scinder les traitements métiers et les données du domaine.
Cela pourrait dans une moindre mesure nous éviter ce problème.

@bashizip
Copy link
Member

Je me sens particulièrement notifié puisque je suis la pardonne qui a créé
la classe NotificationEmail et le scala Qui va avec .

Comme Martial l'a bien dit il s'agit plus de philosophie que de pattern.

Le côté novice en play! me demande de vous laisser la décision .
Mais mon côté avisé en conception me dit qu'il serait plus avisé
d'implémenter un ou deux interfaces pour les notifications.

@martialsomda
Copy link
Member Author

@Centonni Dans la solution que j'ai décrite Il faudrait effectivement aussi répercuter le changement d'interface de la classe EmailNotification sur les templates scala de sorte à ce qu'ils prennent en paramètre des String aussi

@bashizip lol t’inquiète pas, le code du backend est à tous et d'ailleurs si tu regardes l'historique des modifications sur cette classe tu verra que depuis toi il y a eu bien d'autres coupables :-)

@Centonni
Copy link
Member

@martialsomda ok cool! un happy coding en perspective si on veut bien la mettre en pratique cette philosophie :) !

@bashizip et @martialsomda dites moi le projet est dans sa phase finale? j'aurais bien aimé discuter un peu conception avec vous =D !!

@martialsomda
Copy link
Member Author

Oui le projet est dans sa phase finale mais les discussions sont toujours les bien venues, quitte à ce que ça se traduise par une page dans le wiki du projet listant les améliorations possibles en terme de design ou d'architecture..nous ne sommes pas @bashizip et moi les seuls sur ce projet :-) je te propose de suivre les instructions de cette pages https://github.com/JCERTIFLab/jcertif-backend-2013/wiki/Nouvel-Arrivant puis d'écrire au groupe en faisant test propositions.

Tout le monde pourra ainsi réagir

@Centonni
Copy link
Member

^_^ oki!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants