Optimiser mes bidouillages

Eléphanteau du PHP | 24 Messages

04 mars 2011, 10:26

Salut,

Alors je poste ici car je sais qu'il y a plein de gens de bons conseils...
Je fais du php depuis 2 ou 3 semaines là (enorme débutant), mes codes ne buggent pas mais je suppose qu'il y a forcément une meilleure façon de faire que mes bidoullages. (sans passer par la POO dont je saisi à moitié le concept et qui fait que je me retrouve 9 fois sur 10 avec des erreurs pour le moment.).

C'est pourquoi je poste un code simple que je viens de faire concernant un ajout d'image avec un "title" facultatif et la possibilité de supprimer cette dernière.
Très simple en somme mais ça va me permettre, dans l'hypothèse où l'un d'entre vous accepte de me conseiller à ne pas m'embrouiller avec un plat de spagetti trop long si je m'y prend mal.

Je fais ça de tête (parti d'une page blanche...enfin noire ^^ ) sans me référer à quoi que ce soit (pour ma compréhenssion personnelle) donc il se peut que la présentation soit parfois conceptuelle et en suis désolé par avance.
J'ai viré la plupart des div, des id et des class (css) inutiles ici.

En d'autres termes, je cherche surtout à optimiser ma façon de faire via vos conseils car je suis certain que mes pages de test ne tiennent qu'à un fil et qu'il est fort probable que mon chateau de carte tombe au premier souffle. (pas terrible comme métaphore mais j'ai trouvé que ça là :D )
<div>
<form method="post" enctype="multipart/form-data" >
<label for="titre" >Titre <em>(facultatif)</em> : </label><input type="text" name="titre" id="titre" value="<?php if(isset($_POST['titre'])) { echo $_POST['titre']; } ?>" />
<label for"image" >Image <span>*</span> : </label><input type="file" name="image" id="image" />
<input type="submit" name="valid" value="Poster" />
</form>
</div>
<div>
<?php
$actual = date("YmdHis");

// VERIFICATION DES ERREURS
if(isset($_FILES['image']) AND $_FILES['image']['error']==0)
{
	// VERIFICATION DU POID DE L'IMAGE
	if($_FILES['image']['size']<501000)
	{
		$file_info = pathinfo($_FILES['image']['name']);
		$ext_upload = strtolower($file_info['extension']);
		$ext_auto = array('jpg', 'jpeg', 'png', 'gif');
		// VERIFICATION DE L'EXTENSION
		if (in_array($ext_upload, $ext_auto))
		{
			// RENAME DE L'IMAGE PAR LE TIME ACTUEL ET ENVOI DANS LE DOSSIER IMAGE
			$new_name = 'image/'.$actual.'.'.$ext_upload.'';
			// ENREGISTREMENT DE L'IMAGE
			move_uploaded_file($_FILES['image']['tmp_name'], $new_name);
		}
		else { echo '! veuillez sélectionner une image au format .jpg, .jpeg, .png ou .gif et non .'.$ext_upload.'.'; }
	}
	else { echo '! veuillez sélectionner une image de 500ko maximum.'; }
}

try
{
	// CONNEXION A LA BDD
	$pdo_op[PDO::ATTR_ERRMODE] = PDO::ERRMODE_EXCEPTION; $bdd = new PDO('mysql:host=localhost;dbname=test', 'root', '', $pdo_op);
	// VERIFICATION DU POST
	if(isset($_POST['valid']))
	{
		if(empty($_FILES['image']))
		{
			echo '! veuillez poster une image.';
		}
		else
		{
			// TITRE IMPOSE SI VIDE
			if(empty($_POST['titre'])) { $_POST['titre']='image'; }
	
			$titre = strip_tags($_POST['titre']);
			$titre = htmlspecialchars($titre);

                         $titre = preg_replace('#[éèêë]#', 'e', $titre);
	                 $titre = preg_replace('#[àâä]#', 'a', $titre);
	                 $titre = preg_replace('#[ùüû]#', 'u', $titre);
	                 $titre = preg_replace('#[ôö]#', 'o', $titre);
	                 $titre = preg_replace('#[ç]#', 'c', $titre);

                        $titre = strtoupper($titre);

			$new_name = htmlspecialchars($new_name); // pas forcément utile mais bon

			// INSERTION DES INFOS DANS LA BDD
			$prepa = $bdd->prepare('INSERT INTO gallery (image, titre) VALUES (:image, :titre)');
			$inser = $prepa->execute(array('image'=>$new_name, 'titre'=>$titre));
			echo 'l\'image a bien été enregistré dans la base de données.';
			$prepa->closeCursor();
		}
	}
	// SUPPRESSION DES IMAGES DE LA BDD
	if(isset($_POST['delete']))
	{
		$erase = $bdd->exec('DELETE FROM gallery WHERE id=\''.$_POST['id'].'\'');
		echo 'l\'image a été effacé de la base de données.';
		$erase->closeCursor();
	}
	?>
    <div>
    <?php
	// AFFICHAGE DES IMAGES
	$answer = $bdd->query('SELECT * FROM gallery ORDER BY id DESC');
	while($done = $answer->fetch())
	{
		echo '<img src="'.$done['image'].'" width="50" height="50" alt="image" title="'.$done['titre'].'" />';
		?>
        <!-- BOUTON DE SUPPRESSION D'IMAGE -->
        <form method="post" >
        <input type="hidden" name="id" value="<?php echo $done['id']; ?>" />
        <input type="submit" name="delete" value="effacer" />
        </form>
        <?php
	}
	$answer->closeCursor();
	?>
    </div>
    <?php
}
// SI LA BASE DE DONNES ECHOUE
catch (Exception $e) { die ('Echec de la base de données, vérifiez l\'état de votre connexion.' ); }
?>
</div>
J'ai mit que trois champs dans la table 'id, image et titre'.
Voilà, j'attends vos conseils. :priere:

ViPHP
ViPHP | 5462 Messages

04 mars 2011, 11:58

tu devrais faire des pages différentes, une pour l'affichage, une pour le formulaire, une pour le traitement du formulaire,
tu n'as pas a faire de htmlspecialchars pour l'insertion dans une base de données
le code suivant n'es pas protégé et est la pire des injections :
$erase = $bdd->exec('DELETE FROM gallery WHERE id=\''.$_POST['id'].'\'');
les closeCusore sont inutile dans ton cas,
tu mets les messages comme quoi les requêtes sont bien exécuter dans vérifier si ca c'est vraiment bien passer (même si y'a les try...catch),
en procédural je te conseil plus de mettre le mode PDO::ERRMODE_WARNING

Eléphanteau du PHP | 24 Messages

04 mars 2011, 12:46

Merci pour ta réponse.

- Des pages différentes pour chaques parties dans un souci de clareté je suppose, je vais donc essayer de procéder de cette manière.
- C'est vrai qu'il serait plus logique d'insérer des htmlspecialchars pour les affichages. Est-ce exact que stirp_tags ne soit pas vraiment fiable?
- Concernant le DELETE, je comprends le problème mais comment le protéger?
<?php
	if(isset($_POST['delete']))
	{
		$correct_id = $_POST['id'];
		$search = $bdd ->prepare('SELECT COUNT(id) as valid_id FROM gallery WHERE id=:correct_id');
		$go = $search->execute(array('correct_id'=>$correct_id));
		$data = $search->fetch();
		if ($data['valid_id'] =!0)
		{
			$erase = $bdd->exec('DELETE FROM gallery WHERE id=\''.$correct_id.'\'');
			echo 'l\'image a été effacé de la base de données.';
		}
	}
	?>
En écrivant ça je vérifie juste que l'id soit bien présent dans la bdd mais je suppose que ça ne protège pas grand chose au final et là je sèche un peu je dois dire, je ne connais pas de solution pour ce cas ou n'en vois pas.

- Ok pour les closeCursor, je vais regarder dans quel cas précis il est conseillé de les utiliser. (je croyais que c'était à chaques fin de requète db mais visiblement c'est pas ça... on peut lire des ifnos erronés malheureusement sur le net :cry: )
- Merci pour l'ERRMODE_WARNING, je vais donc utiliser ça. Je compte passer tout de même assez vite en POO (je fais des tests qui fonctionnent assez rarement pour le moment), ça m'a l'air plus pratique.

ViPHP
ViPHP | 5462 Messages

04 mars 2011, 12:52

pour le DELETE, sois tu fais une requête préparée, sois il faut protégé la valeur avec PDO::quote
$correct_id = $bdd->quote($data['valid_id']);
le closeCursor sert lorsque tu ne vas pas chercher tout les résultats d'une requête

Eléphanteau du PHP | 24 Messages

04 mars 2011, 13:04

Cool, un cours en direct. :D

Merci pour PDO::quote, je viens de lire la doc du coup pour protéger les chaines avec ça.
Et donc j'ai bien saisi (enfin j'espère) les moments où utiliser closeCursor.
Uniquement après ma boucle d'affichage d'image sur cet exemple en somme.

Comme je vais abuser de vos conseils j'ai juste autre chose à demander pour pouvoir avancer :

Imaginons que je crée une page Essai.class.php avec ceci :
<?php

class Essai
{
	private $texte;
	private $number;
	
	public function __construct()
	{
		$this->texte = 'Texte d\'essai';
		$this->number = 10;
	}
	
	public function afficheInfo()
	{
		return $this->texte. ' ' .$this->number;
	}
	
	public function testBidon()
	{
		$resultat = ($this->number + 9) * $this->number;
		return $resultat;
	}
	
	public static function reTest()
	{
		return 'Ca, ça ne bouge pas !!!<br/>';
	}
}
Et que j'appel cette class dans une autre page
<?php
function chargClass($classe) { require $classe . '.class.php'; } spl_autoload_register('chargClass');

$test = new Essai();
echo Essai::reTest(); // TEST DU STATIC
echo $test->afficheInfo() .' <br/><strong>Résultat</strong> : '.$test->testBidon(); // TEST DES FONCTIONS

?>
Je vois à peu près le fonctionnement même si je m'y suis reprit à trois fois pour ne plus avoir d'erreurs, le fait est que j'ai mal saisi certaines choses:
Comment intégrer mes requètes de db dans une function?
J'ai essayé plusieurs trucs, rien ne fonctionne, j'ai fait un copier/coller d'un exemple trouvé sur le net et bien sûr ça a marché mais je n'ai pas compris la logique de la personne qui a posté ça donc ça me sert à rien.
Si vous pouviez m'éclairer sur le sujet ça serait trop la fête... je demande juste une expliquation pas de code à copier que je ne comprendrais pas.

ViPHP
ViPHP | 5462 Messages

04 mars 2011, 14:28

le plus simple pour PDO étant de faire du Registry ou du Singleton

le Registry va enregistrer toute tes variable/constante, c'est un peux comme l'utilisation du global, mais en mieux

un exemple de class Registry :
class Registry extends ArrayObject
{
    private static $_registry = null;
    
    public static function getInstance()
    {
        if (self::$_registry === null)
        {
           self::$_registry = new self;
        }

        return self::$_registry;
    }
    
    public static function get($index)
    {
        $instance = self::getInstance();
        return $instance->offsetGet($index);
    }
    
    public static function set($index, $value)
    {
        $instance = self::getInstance();
        $instance->offsetSet($index, $value);
    }
}

Registry::set('class', new stdClass());
var_dump(Registry::get('class'));
en version singleton de PDO :
class DB extends PDO
{
    private static $_instance = null;
    
    public static function getInstance()
    {
        $arguments = func_get_args();
        
        if(empty(static::$_instance) || !empty($arguments))
        {
            $class = new ReflectionClass(__CLASS__);
            static::$_instance = $class->newInstanceArgs($arguments);           
        }

        return self::$_instance;
    }
}

$db = DB::getInstance('mysql:host=localhost;dbname=test', 'root');
DB::getInstance()->prepare();

ViPHP
ViPHP | 2577 Messages

04 mars 2011, 15:06

Bonjour,

La principale chose que je remarque dans le script, c'est l'absence de fonctions.

Il est de coutume de considérer qu'un traitement doit tenir sur une page pour être lisible. Découper des traitements en fonction permet de déporter du code ailleurs et de le tester individuellement. Accessoirement, une fonction peut être réutilisée à plusieurs endroit.

23j2cmm
Invité n'ayant pas de compte PHPfrance

04 mars 2011, 16:17

Ok, j'ai prit quelques infos sur ces deux versions et vais me pencher de plus près sur Regitry.
Merci stealth pour ton expliquation.
Ca y est, je vous laisse tranquille jusqu'à mon prochain bloquage :D .