Discussion :

[laurentSc]

Bonjour et bonne année à ceux qui me lisent !

Je m'interroge sur le choix de conception de ma classe Ticket. En gros, je lis des fichiers csv qui contiennent des enregistrements. Exemple :

Ticket ID+;Product Name+;Assigned Group+;Submit Date;Last Resolved Date;Summary;Priority;Status;Submit Month;Resolved month;Region;Country;Submitter;Customer Login ID;Organization;Incident Type;Reported Source

INC000006425994;Phenix;PHENIX_WW_L3;26/12/2018 13:20;17/01/2019 14:23;AR // Florida // Phenix // User unable to complete Phenix installation after Hard Disk change;Low;Closed;Earlier than Jan,19;Jan,19;ISMA;Argentina;SESA408981;SESA29353;International Operations;Incident;Phone

INC000006520388;AutoCAD;AUTOCAD_WW_L2;07/01/2019 14:08;12/02/2019 11:40;ar/florida/Autocad software license issue / Port blocked / communication issues;Medium;Closed;Earlier than Feb,19;Feb,19;ISMA;Argentina;SESA443857;SESA168129;Global Supply Chain;Incident;Direct Input

Donc chaque enregistrement a un identifiant. Il me semblerait plus naturel qu'un objet de cette classe porte sur un seul enregistrement. Si on fait ce choix, une instantiation aurait la forme new Ticket('INC000006425994');. Mais cela nécessite de connaître l'identifiant. Comment récupérer cette information ? Bref, quel serait une bonne conception ? (les 2 lignes blanches de l'exemple, je les ai rajoutées pour une meilleure lisibilité)

[rawsrc]

salut laurentSc,

meilleurs vœux et une santé de fer

Tu lis des fichiers csv, ok. La question est : une fois lus, tu en fais quoi ? Les données sont transférées vers une base de données ? Elles restent comme ça et tu te débrouilles avec des .csv en pagaille ?
Tu dois y apporter des mises à jour...

Bref, il faut être plus précis pour espérer avoir une réponse sur la conception de ton application.

[laurentSc]

Salut rawsrc,

j'ai vu sur mon téléphone que tu avais répondu mais attendu d'être sur mon PC pour la lire et y répondre (vu que le clavier est quand même plus confortable, et au cas où y ait besoin de faire des copier-coller)(et la mise en route est longue).

Donc oui, je mets les données de mes fichiers csv dans une bdd MySql. Pas besoin de les modifier, par contre, il faut pouvoir interroger la table. Voici sa DDL actuelle (que je peux changer si c'est mieux) :

Code sql :

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
 
DROP TABLE IF EXISTS `tickets`;
CREATE TABLE IF NOT EXISTS `tickets` (
  `id` varchar(18) COLLATE utf8_bin NOT NULL,
  `Product_Name` varchar(25) COLLATE utf8_bin DEFAULT NULL,
  `Assigned_Group` varchar(30) COLLATE utf8_bin DEFAULT NULL,
  `Submitted_Date` datetime NOT NULL,
  `Last_Resolved_Date` datetime NOT NULL,
  `Summary` varchar(200) COLLATE utf8_bin NOT NULL,
  `Priority_Key` smallint(2) NOT NULL,
  `Status_Key` smallint(3) NOT NULL,
  `Region` varchar(10) COLLATE utf8_bin DEFAULT NULL,
  `Country` varchar(10) COLLATE utf8_bin DEFAULT NULL,
  `Submitter_SESA` int(6) NOT NULL,
  `Customer_SESA` int(6) NOT NULL,
  `Organization` varchar(30) COLLATE utf8_bin DEFAULT NULL,
  `Incident_Type` varchar(20) COLLATE utf8_bin DEFAULT NULL,
  `Reported_Source` varchar(20) COLLATE utf8_bin DEFAULT NULL,
  PRIMARY KEY (`id`),
  KEY `Prioritytype` (`Priority_Key`),
  KEY `Statustype` (`Status_Key`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

Il y a aussi 2 autres tables Priority et Status

Code sql :

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

1
2
3
4
5
6
7
DROP TABLE IF EXISTS `priority`;
CREATE TABLE IF NOT EXISTS `priority` (
  `id` smallint(2) NOT NULL AUTO_INCREMENT,
  `name` varchar(6) COLLATE utf8_bin NOT NULL DEFAULT '',
  PRIMARY KEY (`id`),
  UNIQUE KEY `name` (`name`)
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

Code sql :

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

1
2
3
4
5
6
7
DROP TABLE IF EXISTS `status`;
CREATE TABLE IF NOT EXISTS `status` (
  `id` smallint(3) NOT NULL AUTO_INCREMENT,
  `name` varchar(13) COLLATE utf8_bin NOT NULL DEFAULT '',
  PRIMARY KEY (`id`),
  UNIQUE KEY `name` (`name`)
) ENGINE=InnoDB AUTO_INCREMENT=7 DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

[rawsrc]

salut,

tu dois viser la simplicité : est-ce que ton ticket a des méthodes particulières que tu devras coder mis à part les classiques accesseurs (set()get()) ?
Si la réponse est non, c'est juste des données extraites de la base pour être affichées, y'a même pas de classe à avoir, un simple tableau de tout, obtenu avec $stmt->fetchAll() suffit amplement.

Il faut éviter de tomber dans le travers de Java par exemple où tout doit être objet. PHP est fortiche avec les tableaux, donc quand il n'y a pas besoin de plus, reste simple.

[laurentSc]

Merci pour ta réponse, mais parfois, il y a d'autres méthodes, quand il s'agit par exemple d'extraire une information à partir de plusieurs attributs. Juste un exemple, mais y en a d'autres : la classe contient 2 attributs SESA_submitter et SESA_customer (SESA est l'identifiant chez Schneider). Déjà, il y a aussi la classe User :

Code sql :

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

1
2
3
4
5
6
7
8
9
10
DROP TABLE IF EXISTS `users`;
CREATE TABLE IF NOT EXISTS `users` (
  `sesa` varchar(10) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL,
  `lastname` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL,
  `firstname` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL,
  `email` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL,
  `sesa_manager` varchar(10) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL,
  `id_site` tinyint(2) NOT NULL,
  PRIMARY KEY (`sesa`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

donc si on veut connaître le site du manager du submitter, on va pas stocker cette information. Si ça se trouve, le même submitter est l'auteur de 50 tickets, donc au lieu de stocker uniquement 50 fois le SESA, ça ferait 50*6 informations à stocker et même plus car du SESA du manager, viennent encore 6 informations, puis du manager du manager 6 autres, etc. En outre on veut aussi savoir si le submitter est le customer (j'avais pensé à une méthode isSubmitterSameThanSubmitter). Il est donc clair qu'il y a plus que de simples getters et setters, et qu'il faut donc faire de la POO...

[laurentSc]

Salut rawsrc,

hier, tu me disais de privilégier la simplicité ; soit. Cela dit, vu les besoins explicités dans le post précédent, je pense qu'on ne peut se passer de la POO. Qu'en penses-tu ? Et si tu es d'accord, vaut-il mieux une classe one_ticket ou une classe all_tickets ? Intuitivement, je choisirais plutôt one_ticket, mais après, pour s'adresser à un ticket, on a besoin de son identifiant, et pour les connaître, il faut lire une colonne entière de la bdd, ce qui ne colle pas avec le concept d'un objet qui serait un ticket unique. Donc, je reste avec ma question : quelle conception ?

[rawsrc]

salut laurentSc,

je t'explique comment je m'organise en général : je divise mon code en Concepts : pour coller à ton exemple : j'aurais une classe Ticket, une autre User. Puis dans chacune d'elles je mets une méthode public function data() qui pointe vers les ressources de base de données paramétrables. Ces ressources que je documente soigneusement ne me renvoient que des tableaux de résultat.
Cela donnerait quelque chose dans ce genre :

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
<?php
 
abstract 
class Concept
{
    public const UID = '';
 
    /**
     * Every concept must be identified with an unique id in the whole application
     * Late static binding
     *
     * @return string
     */
    public static function uid(): string
    {
        return static::UID;
    }
}

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
<?php 
 
use Concept;
 
class Ticket
extends Concept
{
    public const UID = 'ticket';
 
    public function data()
    {
         // ici une classe pointant vers un gestionnaire de ressources de base de données
    }
 
    public function actions()
    {
         // ici une classe pointant vers un gestionnaire d'actions (créer, modifier, supprimer...)
    }
 
}

[laurentSc]

Merci pour ta réponse ; il est trop tard pour que je l'assimile correctement. J'attendrai donc un matin où je suis dispo (pas demain...) ; ça sera plus clair. J'essaierai alors de l'appliquer à ma problématique et j'aurai sûrement des questions.

Néanmoins, j'en ai déjà 2 ce soir :

1- dans ton code de la classe Ticket, tu as 2 méthodes data et actions. Et dans le commentaire, tu mets "ici une classe etc". Classe ou méthode ?

2- tu parles des "ressources de base de données paramétrables". De quoi s'agit-il ?

[laurentSc]

Salut rawsrc,

je fonctionne beaucoup avec les exemples. Pourrais-tu fournir un exemple complet de code d'une de tes implémentations (à mon avis, même sans explications, je comprendrai à l'aide des commentaires). Je pense que avec un exemple, je trouverai la réponse à mes questions.

[laurentSc]

Bonsoir,

j'ai beau avoir pris le temps de réfléchir, je ne vois pas comment démarrer. Pour ne pas faire de la m...., merci de me donner un exemple de ton code ; je crois pouvoir m'en inspirer.

[grunk]

salut laurentSc,

je t'explique comment je m'organise en général : je divise mon code en Concepts : pour coller à ton exemple : j'aurais une classe Ticket, une autre User. Puis dans chacune d'elles je mets une méthode public function data() qui pointe vers les ressources de base de données paramétrables. Ces ressources que je documente soigneusement ne me renvoient que des tableaux de résultat.

D'un point de vue purement conceptuel , je ne suis pas d'accord à avec ca.
Ce n'est pas le rôle de la classe Ticket de récupérer ses données (Information Expert et High cohesion des dessign pattern GRASP).

Pour la partie insertion dans la base je suis pas convaincu qu'une classe Ticket soit nécessaire. Un tableau fournie par la classe qui parse le CSV doit être suffisant. Mais si vraiment on veux une classe , la classe de parsage retournera un tableau de Ticket qui devrait , dans un premier temps en tout cas être un simple POPO (plain old php object).

Pour la partie récupération des données dans la base.
Il doit effectivement exister une classe qui est capable d'aller lire des infos dans la bdd et retourner soit un tableau de données , soit un tableau de Ticket si vraiment c'est indispensable d'avoir une classe. Mais à aucun moment la classe Ticket doit se peupler seule.

[rawsrc]

Ce n'est pas le rôle de la classe Ticket de récupérer ses données (Information Expert et High cohesion des dessign pattern GRASP).

Je me suis mal exprimé ou tu ne m'as pas bien lu, en tout cas je suis bien d'accord avec toi sauf que dans mon cas, je ne fais jamais de POPO. Ma classe n'est pas descriptive au sens où elle listerait des propriétés comme nom, prénom, etc.
Le concept Ticket n'est qu'un concept de haut niveau (abstrait) et pas une représentation des données sous-jacentes.
A la rigueur, il est possible d'avoir à côté une classe TicketData corrélée aux données de la base de données. Mais bon, je ne suis pas fan du POPO, vu la puissance des tableaux en PHP.

[laurentSc]

Bonjour,

ce débat entre spécialistes augmente ma... perplexité. Disons tout de suite que les termes employés (grasp, POPO) ne me parlent pas du tout. (jamais vu auparavant).

Ce que j'ai retenu : une classe pour peupler la bdd à partir des fichiers csv (la méthode pour alimenter la bdd existe déjà ; je la donne pas car y a 117 lignes de code) et une autre classe pour l'interroger. Ai-je bien déduit du débat ?

[grunk]

Oui tu as bien déduis.

La première chose c'est d'avoir des classe par taches :
- parser le csv => une classe
- se connecter à la bdd => une classe
- Gérer les données d'une ou plusieurs table => une ou plusieurs classe

Et non pas une classe qui parse le csv et écris dans la base de données


Et le point où on s'accorde avec rawsrc , c'est qu'une classe pour le plaisir d'une classe ça ne sert à rien. Mieux vaux un jeu de données dans un tableau si la classe n'apporte rien de plus.

Il faut que tes classes aient des responsabilités les plus limitées possibles (responsabilité = tache à réaliser).

Dans ton cas j'aurais tendance à dire , dans un premier temps oublie la classe Ticket , fait un simple tableau.
Si tu te rend compte que tu es en permanence entrain de chercher,manipuler,modifier les données du tableau alors dans ce cas peut être qu'une classe Ticket s'impose pour faciliter le travail sur les données. Et si c'est le tableau que tu modifies tout le temps peut être qu'une classe TicketArray (un tableau de Ticket) sera nécessaire. Mais dans 99% des cas on s'en sort très bien avec un simple tableau et on gagne du coup en simplicité

[laurentSc]

OK, je reconnais dans ton discours la maxime de rawsrc : "diviser pour mieux régner".

La classe de connexion à la bdd, c'est celle de rawsrc .

Je me lance donc là-dedans et reviens avec mes questions.

[laurentSc]

Comme je préfère ne pas aller trop loin sans feedback, je reviens déjà.

D'abord, étant parti sur la POO, et souhaitant garder le plus de choses possibles, je pense continuer avec des classes. J'ai néanmoins créé une classe séparée pour alimenter la bdd. Je pars donc sur 2 classes : csvWrite et Ticket. Comme leur nom l'indique, csvWrite servira à alimenter la bdd et Ticket à récupérer les données dans la bdd. La conception de Ticket me pose des questions.
J'étais parti sur ce choix pour les attributs :

Code php :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
 
    private $id;    //int
    private $product_name;   //string
    private $assigned_group;     //string
    private $submitted_date;    //date
    private $last_resolved_date;    //date
    private $summary;    //string
    private  $priority; //priority
    private  $status;    //status
    private $submitter; //user
    private $customer;  //user
 
    private $done;    //boolean

en commentaire, le type et si pas type primitif, c'est une autre classe. Ayant commencé à écrire quelques méthodes, je m'aperçois que je n'affecte jamais les attributs (sauf $done)

Un exemple :

Code php :

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
public function getProductName(){
$pdo=new MYPDO();
$this->done=0;
?>
<h2>Get Product name (example of "standard" getter)</h2>
<?php
$page = new PhpEcho([DIR_ROOT, 'view Layout.php']);
 
      $body = new PhpEcho([DIR_ROOT, 'view formGetPrdName.php'], []);
      $page['body'] = $body;
 
      // on renvoie au navigateur la page assemblée
      echo $page;
?>
 
 
<?php
if( isset ($_POST['id']) )
{   
    $pdo=new MYPDO();
    $req='SELECT * from `tickets`';//var_dump($req);
    $var=$pdo->select($req,[]);//var_dump($var);
    $req='SELECT product_name from `tickets` where (id=:id)';
    $var=$pdo->select($req,[':id'=>$_POST['id']]);//var_dump($var);
    $this->done=1;
    return($var[0]['product_name']??'0');
}
 
}    //end of method getProductName

En gros, ce getter lit la table et retourne le résultat d'un fetchAll. Etant donné cela, je me demande l'utilité de la POO. Qu'en pensez-vous ?

[laurentSc]

Bonsoir, je relance la discussion : mieux vaut peut-être, comme le suggérait grunk (et rawsrc aussi) un tableau et du procédural (avec quand même la classe de gestion de la bdd et la classe de gestion des vues (PhpEcho)). Peut-être quand même faire du MVC, non ?

[grunk]

Comme leur nom l'indique, csvWrite servira à alimenter la bdd

Comme son nom ne l'indique pas tu veux dire ? une classe CSVWrite pour moi elle va écrire un fichier csv , certainement pas écrire dans une base de données ...
Si le but c'est de lire le contenu d'un CSV et d'écrire ce contenu dans une BDD tu dois alors avoir :
- Une classe CSVParser => lit le contenu du CSV et retourne ce contenu
- Une classe DBWriter => récupère le contenu lu par CSVParseret l'écrit dans la bdd
Les noms sont évidemment purement indicatif
En code ça donnerait un truc du genre :

Code :

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

1
2
3
4
5
$parser = new CSVParser('myfile.csv');
$datas = $parser->parse();
 
$writer = new DbWriter($pdoconnection);
$writer->write($datas);

D'abord, étant parti sur la POO, et souhaitant garder le plus de choses possibles, je pense continuer avec des classes

Faire de la POO ca veux pas dire faire des classes pour tout et n'importe quoi. une classe doit se justifier. Si c'est juste un conteneur de données , comme ca semble être le cas , un tableau est plus adapté et ce n'est pas "mal" ou hors bonne pratiques.

Si vraiment tu veux des objets en requettant ta base de données , tu peux tou à fait faire un fetchobject() avec PDO et lui dire de te retourner une instance de Ticket , mais c'est plus lent et rarement très utile. Voir cet article pour des exemples détaillés

[laurentSc]

Comme son nom ne l'indique pas tu veux dire ?

effectivement, le nom est mal choisi. Et je vois que là où je pensais à une seule classe, tu m'en mets 2 ; responsabilités limitées, tu disais...OK.

Selon moi, il y a un peu plus que seulement regrouper des données ; cf mon post #5.

Au constructeur de CSVParser, tu passes le fichier csv. Mais il faut bien l'uploader quelque part. Pourquoi pas une méthode de cette classe avec un formulaire pour uploader le fichier ?

[grunk]

Parce que le rôle de CSVParser c'est de ... parser, pas d'uploader un fichier.
Donc si tu veux gérer l'upload ,c'est le job d'une autre classe ou éventuellement de ton controller (puisqu'on est en lien direct avec la vue)