Discussion :

[elderion]

Bonjour
Afin d'illustrer mon probleme je vais prendre un exemple tres simple :
J'ai un mini projet de Todolist MVC avec 2 entités: Directeur et Employé.
Le langage n'a pas d'importance, par contre je ne peux pas utiliser de couche ORM.
le Directeur crée les todos et les assigne aux employés qui eux doivent les traiter
Il y a ainsi 2 pages
1. une page avec les états de toutes les todolists (backend Directeur)
2. une page filtrée avec juste les todos de l'employé connecté (frontend)

Ca va paraitre bête, mais j'ai une hésitation coté Model :

1. soit je fais une composition d'objets
entre l'entité et un Repository unique nommé TodosRepository qui contient findAll(), findBy*()
l'entité contient un getter tel que :

Code :

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

1
2
3
4
5
public function todos(){
   if ($this->_todos == null)
     $this->_todos = new TodosRepository($this);
   return $this->_todos;
}

Les rôles sont bien séparés : L'entité expose des actions métier, mais en interne elle appelle le Repo qui lui s'occupe du requetage BDD.

Code :

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

1
2
j'utilise alors soit $entite->todos()->findAll();
soit un wrapper $entite->findAllTodos();

La fonction TodosRepository::findAll() peut etre appelée aussi bien par Directeur que par Employe. En soi ca parait intéressant.
Mais le requetage dépend du type de l'entité : dans le cas où c'est le Directeur qui invoque la fonction, aucun filtrage ne doit s'appliquer. Dans le cas de l'Employer, le filtrage doit s'appliquer.
Pour savoir si le filtre doit s'appliquer ou non je me base sur le type réel de l'objet. Donc la au niveau modélidation objet ya un probleme.
Ou alors findAll() doit attendre un parametre $idEntite;
si $idEntite == null alors pas de filtrage.

2. soit je crée des Repository dédiés
"DirecteurTodos", "EmployeTodos" et chacun sait ce qu'il a a faire, mais c'est dommage de se passer de la composition d'objets

3. soit j'implémente findAll() dans l'Entite.
Du coup le Repositiory n'a plus lieu d'etre mais ya mélange Métier+Persistance

J'ai du mal a voir ce qui est le mieux a faire.
help ! merci d'avance

[elderion]

autre possibilité :

4. Soit je polymorphe la fonction findAll() dans chaque Entite.
Ya pas de classe TodosRepository avec une méthode
prédéterminée et générique findAll() car c'est trop conditionnel pou etre générique :
en effet ya 2 entités qui l'utilisent mais de facon tres différente.

Ainsi $entite->findAll() appelle en interne l'unique requeteur de table Repository et le paramètre avec les filtres dont il a besoin.
Résultat : Repository agit comme un messager qui travaille aupres de la BDD et qui renvoit des data, mais il ne sait pas d'avance quelles data il doit renvoyer.
Il ne connait meme pas l'objet (l'entité) qui le manipule, en revanche l'entité, elle , sait comment manipuler le Repository pour obtenir ce qu'elle veut

Je pense que la 4e est une bonne maniere de faire

qu'en pensez vous ?

[tristan_m]

Bonjour,

Je pense que votre problème se situe dans le fait que vous essayez d'attribuer une responsabilité (obtenir les tâches) à un objet qui ne devrait pas l'avoir (entité directeur ou employé), parce qu'a priori, il ne connait pas l'information (au moins en base de données).

Je pense d'ailleurs que l'entité n'a pas besoin de cette méthode.
En essayant d'analyser votre application, il y a un élément de votre message qui me paraît essentiel :

Il y a ainsi 2 pages
1. une page avec les états de toutes les todolists (backend Directeur)
2. une page filtrée avec juste les todos de l'employé connecté (frontend)

Il indique qu'il y a au moins deux vues séparées, et surtout que l'application propose deux fonctionnalités séparées (une pour les directeurs, l'autre pour les employés), donc deux contrôleurs.
NB : Si une interface administrateur est également prévue, elle nécessiterait probablement aussi une vue et un contrôleur dédiés.

Si on essaye d'identifier les besoins des contrôleurs par rapport au modèle, on en distingue deux : accéder aux tâches "créée par" et accéder aux tâches "attribuée à".

Donc de mon point de vue, l'interactivité proposée aux différents types d'entité se fait principalement au niveau contrôleur.
Au niveau du modèle, Les entités 'Manager' et 'Employee' ne doivent être présentes qu'afin de retourner les infos propres à ces entités (id, nom, fonction, coordonnées, etc..) et éventuellement de vérifier les droits d'accès de l'utilisateur.
Par contre, le modèle doit implémenter un objet TodoRepository qui implémenterait des méthodes getTodosCreatedBy(managerId) et getTodosAssignedTo(employeeId).

Voilà, je ne sais pas si je répond à la question dans le sens où je n'indique pas un des choix proposés. J'espère au moins que ce message vous donnera quelques pistes de réflexion.


Sinon, un peu hors-sujet, considérer que seuls les directeurs peuvent assigner des tâches et seuls les employés peuvent avoir des tâches assignées est assez dangereux.
Que se passe-t-il lorsqu'un 'employé' est promu 'directeur'?
Lorsqu'un 'directeur' reçoit des tâches des ses chefs?
Lorsqu'un 'employé' est jugé suffisamment responsable pour pouvoir créer des tâches mais qu'il n'est pas 'directeur'?
Si c'est une application qui doit perdurer, il vaut mieux privilégier une architecture avec un seul type d'entité (et une seule table) qui aura un attribut 'canCreateTodos'

[Luckyluke34]

Définitivement la 4.

Et encore, il n'y a pas forcément besoin de polymorphisme puisqu'il n'y a apparemment pas de code client qui va appeler todos() sur un objet sans savoir si c'est un Directeur ou un Employé. En effet la liste des tâches et le backend avec les TodoList paraissent deux pages complètement séparées.

En d'autres termes, on peut sans souci avoir employe.todos() dans le premier cas et directeur.allEmployeeTodos() dans le deuxième. Même si on peut objecter au niveau métier que cette méthode n'a pas forcément sa place dans l'objet Directeur (autant il y a une association 1-n claire entre employé et todo, autant le directeur ne possède pas les todo de ses employés...).

La 1. ne me parait pas viable car comme tu dis tu vas devoir faire un if sur le type de l'objet (on foule au pieds le polymorphisme) ou passer en paramètre l'id de l'entité au Repository ce qui est contre-intuitif et sans aucun sens.

La 2. viole un peu le concept de repository puisque l'intérêt est d'avoir un repository par entité et pas un repository par objet appelant.

Sur la 3. tu pointes bien le problème : classe à plusieurs responsabilités, faible cohésion, peu maintenable, sujette à bugs...

Attention aussi au couplage fort entre Employé et Repository lorsque tu fais new TodosRepository($this) (d'ailleurs, pourquoi passer $this ?) Injecter le repository ou à la rigueur un service locator améliorerait la testabilité et la maintenabilité.

[elderion]

Définitivement la 4.
En d'autres termes, on peut sans souci avoir employe.todos() dans le premier cas et directeur.allEmployeeTodos() dans le deuxième. Même si on peut objecter au niveau métier que cette méthode n'a pas forcément sa place dans l'objet Directeur (autant il y a une association 1-n claire entre employé et todo, autant le directeur ne possède pas les todo de ses employés...).

ou alors directeur.employes.allEmployesTodos()
"toutes les todos des employés supervisé par ce directeur"

par contre comme le dit tristant_m il ne faut pas de classe Manager ni Employe mais un type plus abstrait Collaborateur.
Sauf que on sait pas d'avancE le role du collaborateur.
Dans le cas d'un Employé il peut avoir un getter "todos()" mais pas dans l'autre cas.
Ou alors on recup les data du Collaborateur en base, et on le caste en sortie en tant qu'objet de type Role, et de sous-type Manager ou Employé pour garder la cohérence d'implémentation de chacun.

La 1. ne me parait pas viable car comme tu dis tu vas devoir faire un if sur le type de l'objet (on foule au pieds le polymorphisme) ou passer en paramètre l'id de l'entité au Repository ce qui est contre-intuitif et sans aucun sens.

voila c exactement ca.

La 2. viole un peu le concept de repository puisque l'intérêt est d'avoir un repository par entité et pas un repository par objet appelant.

Sur la 3. tu pointes bien le problème : classe à plusieurs responsabilités, faible cohésion, peu maintenable, sujette à bugs...
oui ca pionte beaucoup la question de l'appartenance.

Attention aussi au couplage fort entre Employé et Repository lorsque tu fais new TodosRepository($this) (d'ailleurs, pourquoi passer $this ?) Injecter le repository ou à la rigueur un service locator améliorerait la testabilité et la maintenabilité.

au départ c'etait pour faire de la composition tel que TodosRepository est une classe à qui Employé délègue ses besoins de requetage.
Par injection tu veux dire "injection de dépendance" :
mais ca signifie que a chaque fois que j'instancie un employé, je dois lui passer la référence a TodosRepo ?
a moins que ca soit automatisable dans un framework...?

[Luckyluke34]

par contre comme le dit tristant_m il ne faut pas de classe Manager ni Employe mais un type plus abstrait Collaborateur.

Pourquoi ?

A la rigueur si un Manager a des Todo, pourquoi pas, mais il me parait dangereux de vouloir utiliser un type abstrait à tout prix dès que deux classes se ressemblent de près ou de loin. Ca me rappelle un peu les exemples de livres d'école où Client et Employé héritent forcément de la classe Personne...

Encore une fois, si la classe Collaborateur est une abstraction qui aide d'autres objets à manipuler un Manager et un Employé de façon indifférente (je ne vois pas ce cas dans ton exemple), elle est utile, sinon ça sera probablement source de problèmes...

Par injection tu veux dire "injection de dépendance" :

Oui

mais ca signifie que a chaque fois que j'instancie un employé, je dois lui passer la référence a TodosRepo ?

Soit ça, soit juste passer le repository en paramètre à la méthode.

a moins que ca soit automatisable dans un framework...?

Oui. Par contre je ne connais pas les frameworks d'injection de dépendance qui existent en (PHP ?)

Mettre en place l'injection de dépendance à l'échelle de l'application est un travail de longue haleine qui ne convient pas forcément à tous types d'appli, mais c'est bien de savoir que ça existe et de connaitre les problèmes liés à un couplage fort => classes moins facilement testables unitairement, moins de réutilisabilité, moins de modularité, etc.

Autre solution si tu veux vraiment creuser plus loin : l'approche Domain Driven Design (le pattern Repository vient de là). Dans DDD, les classes métier liées entre elles sont regroupées en agrégats. Lorsqu'on charge depuis la base de données la racine d'un agrégat, on charge tout le graphe d'objets qui en dépendent. Par exemple si je charge un Employé depuis son repository, cela peuple aussi sa variable d'instance qui contient sa liste de Todo donc plus besoin que l'Employé ait accès au repository des Todo pour faire employe.todos(). D'ailleurs, il n'y aurait pas besoin de repository de Todos. Seul la racine de l'agrégat (Employé) a besoin d'un repository.

Cette approche permet de raisonner en petit groupes d'objets liés les uns aux autres qui sont chargés en même temps, supprimés en même temps, etc.

[tristan_m]

Bonjour,

Discussion extrêmement intéressante, j'apprends beaucoup de choses. Merci pour vos réponses détaillées!
J'ai manifestement une approche de l'application très différente de la votre (j'ai probablement tort d'ailleurs )

en fait tu vois les entité comme des DTO, c a dire une structure (les infos), sans méthodes (comportement).

Tout à fait. C'est d'ailleurs comme ça que j'imagine le fonctionnement du modèle entier.
Le rôle du modèle est, pour moi, de définir le fonctionnement de l'environnement de l'application ainsi que le périmètre autorisé. Tout ce qui est pûrement applicatif est dans le contrôleur (il définit le fonctionnement de l'application).

On pourrait résumer la demande telle que : "Je me fiche de savoir comment tu fais, je veux juste que tu me donne l'info que j'attend"

Apparemment vous souhaitez rendre les tâches transparentes du point de vue du contrôleur?
C'est-à-dire ne jamais manipuler de tâches à proprement parler dans le contrôleur?
Vous considérez alors les tâches comme faisant partie de l'environnement de l'application, qui ne seraient modifiées qu'en conséquences d'interactions sur un 'entity'?
Si c'est le cas, je pense que c'est là que se situe notre principale différence de point de vue. Et du coup je n'ai peut être pas compris ce que devait faire l'application, car j'ai un réel problème à considérer les tâches complètement abstraites dans le contrôleur


NB : En attendant un éclaircissement éventuel sur la question précédente, je répond sur les points suivants, en considérant des tâches accessibles au niveau du contrôleur

Sans interactivité dans l'objet, comment faire si un employé veut changer l'etat d'une todo de "en cours" a "terminée"

Le fait qu'un employé décide de changer l'état d'une tâche est typiquement implémenté dans le contrôleur. Pour le modèle, cette opération doit uniquement être une action sur la lecture/modification/création/suppression d'une tâche (qu'il autorise ou non) venant du contrôleur.

Il faut un objet responsable des ACL , qui se situe dans la fonction du Controller de sorte a regulere ce qu'il manipule/

Pour moi les ACL doivent au moins se situer dans le modèle, puisqu'in fine c'est lui qui donne l'autorisation pour effectuer une transaction (lecture ou écriture). Rien n'empêche de faire une première passe dans le contrôleur par contre.

ca signifie que c "open bar" : n'importe quelle entité peut l'appeller librement meme si elle est pas censée d'apres la Domain Logic
on balaie la notion d'appartenance entre l'Entite et les fonctions qui concernent qu'elle.

NB : J'aime bien l'expression "open bar"
Encore une fois, pour moi ce sont des objets "Todos" qui sont manipulés directement au sein du contrôleur. Par contre effectivement, dans le contrôleur on ne peut pas empêcher qu'un employé crée une tâche. Dans ce cas là : Le modèle peut toujours bloquer avec les ACL, ou alors c'est le comportement de l'application (donc défini dans le contrôleur) qui veut ça.

ca donne plus de boulot a l'objet d'ACL : si pas de scope, il faut créer des regles en plus pour pas que
le Controller puisse permettre a un employé de voir les todos d'un autre employé.

A moins qu'il y ait une subtilité au niveau de l'implémentation à laquelle je ne pense pas, ce n'est vrai que si le contrôleur manipule directement l'objet 'entity' du modèle (auquel cas il y aurait une autre divergence de points de vue). Sinon le modèle doit au moins vérifier que l'id de l'utilisateur correspond à l'"entity" indiqué dans l'action envoyée par le contrôleur.

On aurait plutôt une entité Collaborateur qui a le rôle soit de Manager, soit d'Employé, soit de PDG, etc...

Encore une fois, si la classe Collaborateur est une abstraction qui aide d'autres objets à manipuler un Manager et un Employé de façon indifférente (je ne vois pas ce cas dans ton exemple), elle est utile, sinon ça sera probablement source de problèmes...

En fait ce que je voulais dire, c'est que être directeur ou employé c'est une chose, avoir les droits de créer une tâche ou non en est une autre.
Directeur est un rôle au sein de l'entreprise.
Au niveau de l'application, c'est le droit à créer une tâche qui est important.
Relier les deux n'est pas forcément judicieux, et les notions de PDG et autres ne devraient pas être significatives/interprétées dans l'application.
Après, comme le fait remarquer Luckyluke34, il peut y avoir plusieurs classes pour désigner les différents types de collaborateurs, mais par contre j'éviterai d'avoir des classes 'Manager' et 'Employee' qui sont des notions extérieures à l'application.
Personnellement, dans ce cas précis, je confirme mon choix pour une seule classe

• Pour les raisons que j'ai invoquées dans mon message précédent
• Parce que bien sûr, pour moi les tâches ne sont pas manipulées par des 'entity' dans le modèle
• Parce que que mes 'entity' à moi sont "bêtes" .

Merci encore de partager vos arguments fort intéressants!

[elderion]

Bonjour,
Je pense que votre problème se situe dans le fait que vous essayez d'attribuer une responsabilité (obtenir les tâches) à un objet qui ne devrait pas l'avoir (entité directeur ou employé), parce qu'a priori, il ne connait pas l'information (au moins en base de données).

ok tu verrais ca tel que : Controller A qui demande au service TodosRepository les Todos de l'entité B.

Alors que pour moi je trouve plus évident d'interroger l'entité concernée.
C'est elle qui doit savoir répondre a la question "quelles sont mes Todos".
C'est pas juste un container de données (le terme c'est DTO je crois),
mais un objet "intelligent", qui dans sa fonction getMyTodos() etre capable d'interroger TodosRepository
pour recup les données demandées a l'intérieur de la méthode getTodos().
Sinon a quoi sert l'entité, elle n'aurait aucune intelligence ?
Ce qui n'empeche pas d'inverser le décors et d'interroger directement le TodosRepository sans passer par Entite::getMyTodos()

finalement on peut travailler des 2 manieres :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
class EmployeTodoListView
{
    public void EmployeTodoListView(int IdEmploye)
    {
        // IdEmploye : récupéré via GET (http://todoappli.local/employe/1/todos    
        // recup l'Entite employé
        Employer employer = EmployesRepository.instance().findById(IdEmploye);
        // 1re maniere: on demande au Repository les todos de l'entité 
        List<Todo> todoList = TodosRepository.instance().findForEmploye(employe);
        // 2e maniere: on demande a l'employé ses Todolist
        List<Todo> todoList = employe.getTodoList(); 
    }
}

dans le fond les 2 exemples font la meme chose, mais le chemin logique est différent :
- dans le 1er cas on va se demander "quoi et comment obtenir", et donc réfléchir au chemin logique necessaire pour obtenir la TodoList
- dans le 2e cas on va se demander seulement "qu'est ce qu'on veut obtenir"
la question du "comment obtenir" selon moi relève du travail de la fonction getMytodo() de l'entité
qui doit abstraire la manière.
On pourrait résumer la demande telle que : "Je me fiche de savoir comment tu fais, je veux juste que tu me donne l'info que j'attend"
Ca me rappelle une celebre phrase :
"pour faire avancer un chien on lui ordonne d'avancer, on ne lui bouge pas les pattes".

Je pense d'ailleurs que l'entité n'a pas besoin de cette méthode.

nous avons donc 2 points de vue différents. c'est intéressant.

En essayant d'analyser votre application, il y a un élément de votre message qui me paraît essentiel :

Il indique qu'il y a au moins deux vues séparées, et surtout que l'application propose deux fonctionnalités séparées (une pour les directeurs, l'autre pour les employés), donc deux contrôleurs.
NB : Si une interface administrateur est également prévue, elle nécessiterait probablement aussi une vue et un contrôleur dédiés.

Si on essaye d'identifier les besoins des contrôleurs par rapport au modèle, on en distingue deux : accéder aux tâches "créée par" et accéder aux tâches "attribuée à".

en effet il ya 2 actions et donc 2 vues (relation 1:1) :
* pour un Employé : "voir mes todos"
-> + 1 sous action : " changer l'etat d'une todo"
* pour un Directeur : voir l'etat d'avancement de toutes les todos par employé

Donc de mon point de vue, l'interactivité proposée aux différents types d'entité se fait principalement au niveau contrôleur.

Oui less actions utilsateur sont déclenchées a partir de L'IHM et interceptées/traitées par le Controller.
[front controller- > routage -> controlleur.action()]

Au niveau du modèle, Les entités 'Manager' et 'Employee' ne doivent être présentes qu'afin de retourner les infos propres à ces entités (id, nom, fonction, coordonnées, etc..)

en fait tu vois les entité comme des DTO, c a dire une structure (les infos), sans méthodes (comportement).
Sans interactivité dans l'objet, comment faire si un employé veut changer l'etat d'une todo de "en cours" a "terminée"

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

class TodosController extends Controller
{
    public void updateStatus(int idEmploye, int idTodo)
    {
        Employer employer = EmployesRepository.instance().findById(idEmploye);
    
        //toi tu verras plutôt : 
        Todo todo = TodosRepository.instance().findByNum(idTodo);
        todo.status = TodoStatus.DONE;
        todo.todoDoneAt = new Date();
        
        //pour ma part je le vois ainsi :
        employe.todos.findByNum(idTodo)->setStatus(TodoStatus.DONE);
        
        //puis pour mémoriser ce changement en base :
        TodoRepository.instance().save(todo);   // ou persist, ou flush...
        
        // mais pas ca (meme si c'est un raccourci séduisant)  sinon amalgame entre couche Domain et couche Persistance
        - todo.update()
    }    
}

et éventuellement de vérifier les droits d'accès de l'utilisateur.

Ca fait porter 2 responsabilités à l'entité : métier et ACL
Il faut un objet responsable des ACL , qui se situe dans la fonction du Controller de sorte a regulere ce qu'il manipule/

Par contre, le modèle doit implémenter un objet TodoRepository qui implémenterait des méthodes getTodosCreatedBy(managerId) et getTodosAssignedTo(employeeId).

TodosRepository n'est donc plus un DataMapper agnostique avec des méthodes classiques comme findBy*(), findAll(),
mais un Repository dédié a la table des todos avec des méthodes prédéterminées.
En soit l'idée est intéressante, c'est "comme des vues SQL".

Mais ce qui me gene et je pense que c'est ici quse se trouve le flou de mon histoire :
"la question de l'appartenance" :

- getTodosCreatedBy() est une méthode publique du Repository : donc TodosRepository est comme un entrepot de diverses fonctions
applicables a la table des todos.
- ca signifie que c "open bar" : n'importe quelle entité peut l'appeller librement meme si elle est pas censée d'apres la Domain Logic
on balaie la notion d'appartenance entre l'Entite et les fonctions qui concernent qu'elle.
- ca donne plus de boulot a l'objet d'ACL : si pas de scope, il faut créer des regles en plus pour pas que
le Controller puisse permettre a un employé de voir les todos d'un autre employé.
Tandis que si la fonction appartient a Employé, le scope est restreint, le Controlleur peut afficher que
celles attribués à cet employé ci. Point barre, le perimetre est fixé.

Sinon, un peu hors-sujet, considérer que seuls les directeurs peuvent assigner des tâches et seuls les employés peuvent avoir des tâches assignées est assez dangereux.
Que se passe-t-il lorsqu'un 'employé' est promu 'directeur'?
Lorsqu'un 'directeur' reçoit des tâches des ses chefs?
Lorsqu'un 'employé' est jugé suffisamment responsable pour pouvoir créer des tâches mais qu'il n'est pas 'directeur'?
Si c'est une application qui doit perdurer, il vaut mieux privilégier une architecture avec un seul type d'entité (et une seule table) qui aura un attribut 'canCreateTodos'

[/quote]

non c'est pas hors sujet, ta raison c'est au contraire tres important.
Il ne faut pas figer le modele.
La on touche en plein au sujet des ACL :
L'erreur c'est de trop specialiser des types d'objet qui sont constant, qui ne pourront pas evoluer plus tard,
et d'attribuer des droits a ces objets constant.
En fait il faut attribuer des rôles a ces objets et des droits à ces rôles, car un rôle est variable, il peut évoluer
aussi bien que l'assignation des droits.

Tu as raison : Employé n'est pas une entité en soi mais un rôle.
On aurait plutôt une entité Collaborateur qui a le rôle soit de Manager, soit d'Employé, soit de PDG, etc...
On peut ainsi décrire un organigramme d'entreprise dans une matrice de configuration des ACL.
Comme le fait un Active Directory en somme : on gère des groupes (des roles), leurs droits,
on met des utilisateurs dans ces groupes pour leur donner tel role, et tout ca évolue.
Les utilisateurs du réseau de l'entreprise en eux meme restent inchangés. c'est leur accès aux ressources qui change.

Par contre Collaborator.canCreateTodo() serait a mon avis un wrapper ves l'objet qui gere les ACL.