Discussion :

[ypicot]

Bonjour,

Une question d'organisation du code :

Soit un fichier CSV à importer.
Ce fichier étant utilisé par des humains, il faut faire un max de vérif avant de l'importer.

L'import se subdivise en plusieurs fonctions :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
// fonction principale.
/**
 * $sourceFile : nom du fichier source
 * $destTable : nom de la table dans laquelle seront importées les données
 * $fields décrit la structure attendue
 */
function importCsv($sourceFile, $destTable, $fields) {
  list($header, $lines) = getCsvFile($sourceFile, $fields); // fait l'import brutal du fichier, et récupère d'un coté le header (où se trouvent les champs), et de l'autre les données elles-mêmes
  checkHeader($header, $fields); // vérifie les données du header 
  checkLines($lines, $fields); // vérifie les données des lignes
 ...
}

Maintenant, je veux afficher un joli msg d'erreur avec notamment le nom du fichier incriminé, je suis obligé de passer $sourceFile dans les différentes sous-fonctions (checkHeader et checkLines).

J'envisage plusieurs solutions :
- passer le nom du fichier fautif $sourceFile en paramètre. Or, ce paramètre n'est pas réellement nécessaire aux fonctions. Pas beau.
- intégrer le tout dans un objet, et mettre $sourceFile en variable membre. On appelle la "fonction" par le constructeur (ou la méthode statique).
On se balade donc avec un $this->sourceFile (ou un self::$sourceFile), mais surtout on utilise un objet à contre-emploi (à mon sens)

La meilleure solution (les fonctions incluses l'une à l'intérieur de l'autre, qu'on retrouve notamment en Pascal) n'existe pas en PHP

Détail : j'ai volontairement pris un cas simple. Il existe certains cas ou je me retrouve avec 3 ou 4 "paramètres" en plus, ce qui alourdi considérablement toutes les écritures.

Auriez-vous une autre approche plus élégant me suggérer ?

Comment faites-vous dans ce genre de cas ?

Yvan

[Madfrix]

Bonjour,

quelque chose me chiffonne en voyant ton code :

Code php :

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

1
2
3
4
5
6
7
 
function importCsv($sourceFile, $destTable, $fields) {
  list($header, $lines) = getCsvFile($sourceFile, $fields); // fait l'import brutal du fichier, et récupère d'un coté le header (où se trouvent les champs), et de l'autre les données elles-mêmes
  checkHeader($header, $fields); // vérifie les données du header 
  checkLines($lines, $fields); // vérifie les données des lignes
 ...
}

car ensuite tu parles de $this->sourceFile ou self::$sourceFile mais ton organisation de code se fait elle en procédural ou en objet ?

[ypicot]

Le code actuel est procédural, car je ne vois pas l'intérêt de mettre de l'objet dans ce cas-là.

J'avais essayé une approche objet, avec $sourceFile et autres "paramètres" en variables membres, mais le résultat me semble lourd.
En effet, il n'y a pas de raison de mettre un objet : conceptuellement, l'import est vu comme une fonction qui est appelé (en une fois), qui fait son boulot qui retourne un résultat.

Yvan

[Madfrix]

Je trouve l'approche objet relativement appropriée moi au contraire. Un objet fichier autour du quel gravitent les méthodes de vérification du header des lignes etc avec génération d'une propriété d'un tableau d'erreur

[ypicot]

Pour l'appel, cela donnerait donc qque chose du genre :

Code :

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

1
2
3
4
...
$dummy = new ImportCsv($sourceFile, $destTable); // tout est fait dans le constructeur
$dummy = NULL;
...

Ou éventuellement :

Code :

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

1
2
3
4
...
$myImport = new ImportCsv($sourceFile, $destTable); // qques init
$myImport->doImport(); // import en lui-même
$myImport = NULL;

C'est là que je ne suis pas convaincu : on créé un objet qui n'est jamais utilisé "à l'extérieur".

A moins de créer un objet entièrement statique...

Yvan

[Madfrix]

Effectivement, l'objet n'est pas utilisé à l'extérieur du moins avec ce que tu nous as montré comme méthode mais quoi qu'il en soit l'objet sera toujours une manière plus 'élégante' je trouve

[Petibidon]

Juste un truc en reprenant l'exemple original :

Code :

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

1
2
3
4
5
6
7
 
function importCsv($sourceFile, $destTable, $fields) {
  list($header, $lines) = getCsvFile($sourceFile, $fields); // fait l'import brutal du fichier, et récupère d'un coté le header (où se trouvent les champs), et de l'autre les données elles-mêmes
  checkHeader($header, $fields); // vérifie les données du header 
  checkLines($lines, $fields); // vérifie les données des lignes
 ...
}

Les fonctions checkHeaders et checkLines vérifient des trucs et quoi ? Rien ?
La vocation d'une fonction de vérif, souvent, c'est de renvoyer un booleen représentant le résultat de la vérification, non ?

Par exemple :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
 
function importCsv($sourceFile, $destTable, $fields) {
  list($header, $lines) = getCsvFile($sourceFile, $fields); // fait l'import brutal du fichier, et récupère d'un coté le header (où se trouvent les champs), et de l'autre les données elles-mêmes
  if( !checkHeader($header, $fields)){
      echo "y'a un truc qui cloche dans le header du fichier " . $sourceFile;
      return false;
  } 
  if( !checkLines($lines, $fields) ){
      echo "y'a un truc qui cloche dans les données du fichier " . $sourceFile;
      return false;
  }
 ...
}

[ypicot]

Madfrix, merci pour cet éclairage. Je sens que je vais créer un objet statique (cad composé d'une seule méthode statique publique, et de méthodes statiques privées).

Petibidon, dans l'exemple de code que j'ai donné, les erreurs sont levées dans le check, puis loguées dans la gestion d'erreur.
Comme c'est un traitement batch, la règle est "s'il y a un soucis, on arrête tout".

En gros :

Code :

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

1
2
3
4
5
6
    static function __checkHeader($header) {
        if (count($header)!=count(self::$fields)) {
            // the number of fields in the header isn't the expected one
            throw new YpException(self::$sourceFile.' : wrong header', E_USER_ERROR);
        } 
    ...

Puis le gestionnaire d'erreur :

Code :

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

1
2
3
4
5
6
7
8
9
10
class YpException extends Exception {
    public function __construct($message, $level) {
        switch ($level) {
            case E_USER_ERROR:
                $message = date('d/m/Y-H:i:s', time()).' =>ERROR   => '.$message."\n";
                error_log($message, 3, LOG_FILE);
                error_log($message, 1, ADMIN_EMAIL);
                die(MSG_ERR_MAIN);
                break;
...