Discussion :

[Hadafang]

Bonjour à tous,

Je travail actuellement sur de la factorisation de code, exercice que je trouve particulièrement difficile, et je galère totalement. J'ai créer un petit script qui permet d'ajouter des produits dans un panier et de les supprimer. Le code fonctionne très bien, mais il n'est pas vraiment propre, et dans un soucis d'optimisation, je dois le transformer en une fonction. Mais je n'ai pas l'habitude de faire ça et je ne comprend pas comment je pourrais m'y prendre.

Voici le code en question :

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
if (isset($_POST) && !empty($_POST)) {
 
    $post = $_POST;
 
    if(!isset($action)){
        $tab = [$_POST["product_id"] . "a" => ["qt" => $_POST["qt"]]];
 
        if (existPanier($post, $panier)) {
            $index = $_POST["product_id"] . "a";
            unset($panier[$index]);
            $tab2 = array_merge($panier, $tab);
            $test = $ordersModel->addPanier($user->id,serialize($tab2));
            redirect("/commandes");
        } else {
            $tab2 = array_merge($panier, $tab);
            $test = $ordersModel->addPanier($user->id,serialize($tab2));
            redirect("/commandes");
        }
    }
 
    if(isset($_POST['delete_product'])) {
 
        if (existPanier($post,$panier)){
            $index = $_POST["product_id"] . "a";
            unset($panier[$index]);
            $test = $ordersModel->addPanier($user->id,serialize($panier));
            redirect("/commandes/panier");
        }
    }
 
}

Je ne comprend pas bien comment je dois m'y prendre pour simplifier tout ça étant donné que je fais déjà appel à des fonctions dans ce code, votre aide est la bienvenue ! Merci d'avance !

[rawsrc]

salut,

houlà !
tu peux poster le code de ta fonction existPanier(), stp

[Hadafang]

Oui bien sur ! La voici :

Code :

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

1
2
3
4
5
function existPanier($post, $panier)
{
    return isset($post["product_id"]) && array_key_exists($post["product_id"] . "a", $panier);
 
}

[rawsrc]

d'où provient $action ?

[Hadafang]

Cela provient d'une fonction qui fait le travail d'un écouteur d'événement, il détecte si le $_POST est soumis et exécute le reste du script.

[rawsrc]

tu gères un écouteur d’événement et tu as du mal à factoriser ton code ?!?? Ce n'est pas très cohérent. M'enfin...

[rawsrc]

toute ta tartine de code se résume à :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
if (isset($_POST['product_id'])) {
    $id  = $_POST['product_id'].'a';
    $qte = isset($_POST['qt']) ? (int)$_POST['qt'] : 0;   // ici je suppose que les quantités sont des entiers
 
    if (isset($_POST['delete_product']) || ($qte <= 0)) {
        unset($panier[$id]);
        $redirect = '/commandes/panier';
    } else {
        $panier[$id] = $qte;
        $redirect    = '/commandes';
    }
    $test = $ordersModel->addPanier($user->id, serialize($panier));
    redirect($redirect);
}

et il n'y a plus besoin de ta fonction existPanier() qui ne sert à rien à vrai dire

[Trehinos]

Pour refactoriser n'importe quel bout de code en fonction (il faut que le bout de code ait une certaine cohérence), commencez par analyser ce que fait votre code et mettre des mots dessus.

Visiblement ce code ajoute/retire ou met à jour la quantité d'un article dans un panier. On peut donc la nommer majPanier(), par exemple (ou updateCart() mais on évitera de mélanger anglais et français pour plus de cohérence).

Ensuite, on analyse les entrées et les sorties. En sortie, le code redirige... vers une URL. Donc la sortie sera un string qui contient l'URL de redirection (on ne redirige pas dans la fonction pour que celle-ci n'ait vraiment pour responsabilité que de mettre à jour le panier. Une responsabilité par fonction, c'est plus simple (principe KISS, SRP et SOLID)).
Les entrées : il faut le panier à modifier, l'ID du produit et la quantité. Là aussi on pourrait récupérer les informations depuis $_POST mais ce serait contrevenir aux bonnes pratiques citées plus haut.
On passe le panier par référence car il est modifié dans la fonction. Et on ajoute un booléen pour pouvoir forcer la suppression comme dans le code d'origine (par défaut à faux).

Le respect des bonnes pratiques permet d'utiliser la fonction comme prévu mais étend aussi son usage : on peux l'utiliser avec n'importe quelles variables (pas forcément $_POST) et on n'est plus obligé de rediriger si jamais plus tard on veut exécuter la même fonction suivie d'autres actions.

Ainsi donc, on aurait la fonction (je pars du code réécrit de rawsrc qui a déjà bien "factorisé" le code) :

Code :

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

1
2
3
4
5
6
7
8
9
function majPanier(array &$panier, int $idProduit, int $qte, bool $forceDelete = false): string {
    if ($forceDelete || $qte <= 0) {
        unset($panier[$idProduit]);
        return '/commandes/panier';
    }
 
    $panier[$idProduit] = $qte;
    return '/commandes';
}

Et on peut l'appeler ainsi :

Code :

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

1
2
3
4
5
6
7
8
9
$id = ($_POST['product_id'] . 'a') ?? null;
if ($id !== null) {
    $qte = intval($_POST['qt'] ?? 0);
    $forceDelete = isset($_POST['delete_product']);
    $url = majPanier($panier, $id, $qte, $forceDelete);
 
    $test = $ordersModel->addPanier($user->id, serialize($panier)); // à l'extérieur de la fonction sinon on doit lui passer $test, $ordersModel et $user, juste pour cette opération... Ajouter dans la fonction avec les paramètres supplémentaires si nécessaire.
    redirect($url);
}

[Hadafang]

Merci beaucoup pour ces explications très clair ! Je vais essayer de le refaire de moi même en suivant vos explications, c'est comme ça qu'on comprend et qu'on apprend !

Pour l'écouteur d'événement, je l'avais déjà fais sur un projet de groupe, j'ai juste repris et adapté à mes besoins

Encore merci pour le temps que vous m'avez accordé et pour votre aide très précieuse !