2010-08-16 5 views
2

L'utilisateur 2 vous propose d'acheter un article de Utilisateur 1. L'utilisateur 1 peut accepter ou rejeter. Si utilisateur 1accepte, alors ils seront tous les deux en mesure d'offrir des commentaires sur la transaction.Lequel de ces deux blocs IF est le meilleur codage?

J'ai 2 blocs d'instructions IF. Ils travaillent tous les deux et font la même chose mais quelle est la meilleure pratique de codage?

SI BLOC 1 vérifie si quel utilisateur est-il d'abord, puis vérifie si la transaction a été acceptée ou si son toujours en attente

if ($_SESSION['user_id'] == $seller) { 

    if ($row['status'] == 'P') { 
     echo '<p>' . get_username_by_id($row['buyer']) . ' has made a bid of ' . $row['price'] . ' for your ' . $row['title'] . ' 
    <a href="transactions.php?id=' . $transactionid . '&action=accept">Accept</a>/<a href="transactions.php?id=' . $transactionid . '&action=reject">Reject</a><br />'; 
    } else if ($row['status'] == 'A') { 
     echo '<p>' . get_username_by_id($row['buyer']) . ' paid ' . $row['price'] . ' for your ' . $row['title'] . '</p>'; 
     echo '<a href="feedback.php?id=' . $transactionid . '&action=givefeedback">Give Feedback</a></p>'; 
    } 
} else if ($_SESSION['user_id'] == $buyer) { 

    if ($row['status'] == 'P') { 
     echo '<p> You have made a bid of ' . $row['price'] . ' for ' . $row['title'] . '</p>'; 
    } else if ($row['status'] == 'A') { 
     echo '<p> You have paid ' . $row['price'] . ' for ' . $row['title'] . '</p>'; 
     echo '<a href="feedback.php?id=' . $transactionid . '&action=givefeedback">Give Feedback</a></p>'; 
    } 
} 

Ou

IF BLOC 2 a seulement 4 instructions if et vérifie à la fois l'utilisateur et le statut de la transaction en même temps

if ($_SESSION['user_id'] == $seller && $row['status'] == 'P') { 
    echo '<p>' . get_username_by_id($row['buyer']) . ' has made a bid of ' . $row['price'] . ' for your ' . $row['title'] . ' 
    <a href="transactions.php?id=' . $transactionid . '&action=accept">Accept</a>/<a href="transactions.php?id=' . $transactionid . '&action=reject">Reject</a><br />'; 
} else if ($_SESSION['user_id'] == $buyer && $row['status'] == 'P') { 
    echo '<p> You have made a bid of ' . $row['price'] . ' for ' . $row['title'] . '</p>'; 
} else if ($_SESSION['user_id'] == $seller && $row['status'] == 'A') { 
    echo '<p>' . get_username_by_id($row['buyer']) . ' paid ' . $row['price'] . ' for your ' . $row['title'] . '</p>'; 
    echo '<a href="feedback.php?id=' . $transactionid . '&action=givefeedback">Give Feedback</a></p>'; 
} else if ($_SESSION['user_id'] == $buyer && $row['status'] == 'A') { 
    echo '<p> You have paid ' . $row['price'] . ' for ' . $row['title'] . '</p>'; 
    echo '<a href="feedback.php?id=' . $transactionid . '&action=givefeedback">Give Feedback</a></p>'; 
} 
+0

double possible de [Nested par rapport aux conditions composites] (http://stackoverflow.com/questions/3378629/nested-versus-composite-conditions) – Gordon

+3

Je m'intéresserais beaucoup plus au mishmash laid de la concaténation HTML et de la logique de flux de contrôle PHP. – Gordon

+0

@Gordon merci pour le lien – Jonathan

Répondre

4

Le premier vous montre le chemin: si les états devaient augmenter en nombre, vous pourriez résumer la fonctionnalité dans une fonction avec peu de travail. Il a l'air et est plus propre.

Je préférerais aussi séparer le HTML actuel de PHP. Au lieu de

echo '<p>' . get_username_by_id($row['buyer']) . ' has made a bid of ' 
. $row['price'] . ' for your ' . $row['title'] . ' 
<a href="transactions.php?id=' . $transactionid . '&action=accept">Accept</a>/
<a href="transactions.php?id=' . $transactionid . '&action=reject">Reject</a><br />'; 

Je préfère

<p> 
    <?= get_username_by_id($row['buyer']) ?> has made a bid of 
    <?= $row['price'] ?> for your <?= $row['title'] ?> 
    <a href="transactions.php?id=<?= $transactionid ?>&action=accept">Accept</a>/
    <a href="transactions.php?id=<?=$transactionid?>&action=reject">Reject</a> 
</p> 

Mais à chacun son propre.

+0

Note: J'ai utilisé des étiquettes courtes pour économiser de l'espace, je sais qu'ils ne sont pas toujours actifs sur tous les serveurs. Si c'est le cas, remplacez

3

Ce sujet est à l'opinion, mais je suis sûr que la plupart des gens seront d'accord que le premier est plus propre parce que vous supprimez la duplication (la vérification pour voir si elles sont acheteur ou vendeur). Ce sera encore plus évident si vous aviez plus de types status.

1

Je dirais que le bloc 1 est une meilleure pratique de codage en général, puisque vous ne dupliquez pas d'informations. Cela dit, le Bloc 2 décrit avec plus de précision l'ensemble des objets à motifs de Stratégie qui pourraient survenir dans un environnement linguistique différent et qui pourraient réduire davantage la complexité de votre code.

1

Par choix personnel, je préfère la structure de l'option 1 en raison du nombre réduit de conditions. Mais je voudrais changer chaque else if à elseif pour éviter les erreurs en raison d'accolades omis.

Pour que le spectacle de code certaines données commun est utilisé dans la production, et les différences de la fermeture </p> étiquette de chaque choix, je le changer en quelque chose comme ceci:

$buyerName = get_username_by_id($row['buyer']); 
$price = $row['price']; 
$title = $row['title']; 

if ($_SESSION['user_id'] == $seller) { 
    if ($row['status'] == 'P') { 
     echo "<p>$buyerName has made a bid of $price for your $title" 
      . " <a href='transactions.php?id=$transactionid&amp;action=accept'>Accept</a> /" 
      . " <a href='transactions.php?id=$transactionid&amp;action=reject'>Reject</a><br />"; 
    } elseif ($row['status'] == 'A') { 
     echo "<p>$buyerName paid $price for your $title</p>" 
      . "<a href='feedback.php?id=$transactionid&amp;action=givefeedback'>Give Feedback</a></p>"; 
    } 
} elseif ($_SESSION['user_id'] == $buyer) { 
    if ($row['status'] == 'P') { 
     echo "<p> You have made a bid of $price for $title</p>"; 
    } elseif ($row['status'] == 'A') { 
     echo "<p> You have paid $price for $title</p>" 
      . " <a href='feedback.php?id=$transactionid&amp;action=givefeedback'>Give Feedback</a></p>"; 
    } 
}