Discussion :

[simsimz]

Bonjour à tous, étant débutant j'ai besoin de votre aide sur un sujet :
SonarQube ( logiciel qui mesure la qualité du code ) me demande d'utiliser une constante au lieu de dupliqué 3 fois ( $_GET["docname"] ) sur le code suivant :

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
<?php
 
$servername = "http://blabla";
 
if( isSet( $_GET["docname"] ) ) {
	$path = $servername;
 
	switch( strtolower($_GET["docname"]) ) {
 
		case 'guide_tarifaire_res':
			$path .= "/path/to/something";
			break;
 
		case 'guide_tarifaire_pro':
			$path .= "/path/to/something";
			break;
 
		default:
			header("location: http://www.exemple.com");
			break;
	}
	$pdfname = $_GET["docname"].".pdf";
	$pdf = file_get_contents($path);
	$len = strlen($pdf);
 
 
    header("Content-type: application/pdf");
	header("Content-Length: $len");
	header("Content-Disposition: inline; filename=$pdfname");
	echo $pdf;
} else {
	header("location: http://www.exemple.com");
}
 
?>

Dois-je faire une constante comme ceci :

Code :

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

define("DOCNAME", $_GET["docname"]);

et ensuite l'appelée comme ceci :

Code :

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

if( isSet(DOCNAME) )

et

Code :

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

$pdfname = DOCNAME.".pdf";

?

Ou bien niveau performance serait-il plus judicieux d'utilisé

Code :

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

const DOCNAME = $_GET["docname"];

mais il me semble que c'est possible uniquement pour une classe, ce qui n'est pas mon cas.

Ou bien vu que je redéfinis dans le switch la variable en question , une constante n'est pas appropriée et je devrais utilisé une variable classique ?

Merci d'avance !

[Dendrite]

Moi je vote simple variable.
Une constante en PHP n'a (à mon avis) vraiment de sens que dans un fichier de config que l'on incluerait partout...
Là, le correcteur de code te dit juste que tu peux ranger ton $_GET, variable d'environnement, dans une variable tout court...

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
<?php
 
$path = "http://blabla";
 
if( ! empty( $_GET["docname"] ) ) {
  $docname=strtolower($_GET["docname"]));
 
	switch($docname) {
 
		case 'guide_tarifaire_res':
			$path .= "/path/to/something";
			break;
 
		case 'guide_tarifaire_pro':
			$path .= "/path/to/something_else";
			break;
 
		default:
			header("location: http://www.exemple.com");
      break;
	}
	$pdfname = $docname.".pdf";
	$pdf = file_get_contents($path);
	$len = strlen($pdf);
 
 
  header("Content-type: application/pdf");
	header("Content-Length: $len");
	header("Content-Disposition: inline; filename=$pdfname");
	echo $pdf;
} else {
	header("location: http://www.exemple.com");
}
 
?>

A part ça, si tu tenais à une constante, en PHP, c'est ainsi qu'on les manipule :

Code :

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

1
2
3
4
5
6
7
8
9
10
 
//dans un fichier de config
define('MAX_JEUX',10);
//sur n'importe quel script
include('config.php');
if(defined('MAX_JEUX')){
   for($i=1;$i<=MAX_JEUX;$i++){
      echo 'Tu peux encore jouer !<br/>';
   }
}

[rawsrc]

Salut,

un logiciel de mesure de qualité du code qui te conseille de remplacer une valeur en provenance d'un tableau super global comme $_GET par une constante, est bon à jeter à la poubelle.

Par contre, il ne te dit pas comment améliorer ton code par 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
18
19
20
21
22
23
<?php
 
$servername = "http://blabla";
$paths      = [
    'guide_tarifaire_res' => "/path/to/something",
    'guide_tarifaire_pro' => "/path/to/something" 
];
 
if (isset($_GET['docname'], $paths[$_GET['docname']])) {
    $path    = $servername.$paths[$_GET['docname']];
    $pdfname = $_GET["docname"].".pdf";
    $pdf     = file_get_contents($path); // t'es sûr que ton fichier existe au moins ?
    $len     = strlen($pdf);
 
    header("Content-type: application/pdf");
    header("Content-Length: {$len}");
    header("Content-Disposition: inline; filename={$pdfname}");
    echo $pdf;
} else {
    header("location: http://www.exemple.com");
    exit;   // toujours mettre un exit; après un header(location:)
}
// ne jamais fermer la balise <?php quand un fichier ne contient que du code PHP

[Celira]

Ce serait sans doute une bonne idée de lire l'erreur exacte dans Sonar (qui est souvent accompagnée d'un exemple pour mieux comprendre ce qui cloche), plutôt que de partir dans toutes les directions.
En l'occurrence, ce qui pose problème à Sonar, c'est la duplication d'une chaine, pas d'une variable.
Donc, ce qu'il attend, c'est quelque chose : define("DOCNAME", "docname"); à l'utiliser par : $_GET[DOCNAME].
L'idée étant que si tu veux changer la valeur de cette chaine, il est plus simple de changer la valeur de la constante à un seul endroit (sans changer le nom de la constante) plutôt que d'avoir à changer la chaine partout dans le code et risquer d'en oublier une.

String literals should not be duplicated

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

https://rules.sonarsource.com/php/RSPEC-1192