Какой из этих двух блоков IF является лучшей практикой кодирования? - PullRequest
2 голосов
/ 16 августа 2010

Пользователь 2 предлагает купить товар у Пользователь 1 . Пользователь 1 может принять или отклонить .Если Пользователь 1 принимает , они оба смогут предоставить отзыв о транзакции.

У меня есть 2 блока операторов IF. Они оба работают и выполняют одно и то же, но что является лучшей практикой кодирования?

IF BLOCK 1 проверяет, есть ли первый пользователь, а затем проверяет,транзакция была принята или если она все еще находится на рассмотрении

 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>';
    }
}

или

IF BLOCK 2 имеет только 4 оператора if и проверяет как пользователя, так и пользователя.статус транзакции одновременно

   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>';
}

Ответы [ 4 ]

4 голосов
/ 16 августа 2010

Первый показывает вам путь: если число статусов увеличилось, вы могли бы абстрагировать функциональность в функцию без особых усилий.Это выглядит и чище.

Я бы также предпочел отделить реальный HTML от PHP.Вместо

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 />';

Я бы предпочел

<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>

Но каждому свое.

3 голосов
/ 16 августа 2010

Это зависит от мнения, но я уверен, что большинство людей согласятся с тем, что первый из них чище, потому что вы удаляете дублирование (проверьте, являются ли они покупателем или продавцом).Это станет еще более очевидным, если у вас будет больше status типов.

1 голос
/ 16 августа 2010

В качестве личного выбора я предпочитаю структуру варианта 1 из-за меньшего количества условий. Но я бы изменил каждый else if на elseif, чтобы избежать ошибок из-за пропущенных скобок.

Чтобы код показывал, что в выводе используются некоторые общие данные, и различия в закрывающем теге </p> каждого выбора, я бы изменил его на что-то вроде этого:

$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>";
    }
}
1 голос
/ 16 августа 2010

Я бы сказал, что блок 1 - это лучшая практика кодирования в целом, поскольку вы не дублируете информацию. При этом Блок 2 более точно описывает набор шаблонных объектов Стратегии, которые могут возникнуть в другой языковой среде и которые могут еще больше снизить сложность вашего кода.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...