Я злоупотребляю функцией GOTO здесь?
Я строю CRM для своей жены и я буду использовать для нашего бизнеса. Я создал страницу с несколькими целями:
- Уметь создавать новую запись в базе данных.
- Иметь возможность просматривать существующую запись в базе данных.
- Уметь обновлять существующую запись в базе данных.
Первоначально у меня было несколько php-файлов, выполняющих этот материал, но теперь я использовал функцию GOTO, чтобы заставить код перебрасываться по разным частям, которые мне нужно запустить, в зависимости от того, что происходит все время, оставаясь на той же странице.
Мой вопрос, кроме того, что это выглядит грязно, есть ли падение, чтобы сделать это таким образом? В будущем я буду искать другие и более чистые способы сделать это (предложения приветствуются), но на данный момент это работает для меня, и я хотел бы продолжить работу над проектом и приступить к созданию дополнительных частей, которые мне требуются для CRM. Думайте об этом как о бета-версии, если хотите. Если к тому, что я уже сделал, есть какой-то огромный недостаток, я бы скорее обратился к нему сейчас, но если это хотя бы слегка разумно, я буду продвигаться вперед.
Вот что у меня есть:
<?php
// Include Connection Credentials
include("../../comm/com.php");
//Connection to Database
$link = mysqli_connect($servername, $username, $password, $dbname);
// Connection Error Check
if ($link->connect_errno) {
echo "Sorry, there seems to be a connection issue.";
exit;
}
// Define Empty Temporary Client ID
$new_client_id ="";
// Define Empty Success Message
$successful ="";
// Define Empty Error Messages
$firstnameErr ="";
$lastnameErr ="";
$addressErr ="";
$cityErr ="";
$stateErr ="" ;
$zipcodeErr ="";
$phoneErr ="";
$emailErr ="";
// CHECK FOR SEARCH PROCESS
if ($_SERVER["REQUEST_METHOD"] == "POST") {
if (isset($_POST['searched'])) {
$client_id = $_POST['client_id'];
$buttontxt = "Update";
goto SearchReturnProcess;
}
}
// Retrieve Client ID
if ($_SERVER["REQUEST_METHOD"] == "POST") {
if (empty($_POST['client_id'])) {
$buttontxt = "Create Client";
goto CreatNewClientProcess;
} else {
$client_id = $_POST['client_id'];
$buttontxt = "Update";
goto UpdateClientProcess;
}
}
// CONTINUE FOR NEW CLIENT
CreatNewClientProcess:
// Check For Missing Fields and report
if ($_SERVER["REQUEST_METHOD"] == "POST") {
if (empty($_POST["firstname"])) {
$firstnameErr = "First name is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["lastname"])) {
$lastnameErr = "Last name is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["email"])) {
$emailErr = "Email is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["phone"])) {
$phoneErr = "Phone is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["address"])) {
$addressErr = "Address is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["city"])) {
$cityErr = "City is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["state"])) {
$stateErr = "State/Province is a required field - please make entry below";
goto FinishUpProcess;
}
if (empty($_POST["zipcode"])) {
$zipcodeErr = "Postal code is a required field - please make entry below";
goto FinishUpProcess;
}
}
// Prepared Statement For Database Search
if ($stmt = $link->prepare("INSERT INTO client (firstname, lastname, address, city, state, zipcode, phone, email) VALUES (?,?,?,?,?,?,?,?)")){
// Bind Search Variable
$stmt->bind_param('ssssssss', $firstname, $lastname, $address, $city, $state, $zipcode, $phone, $email);
// Define Form Field Input
$firstname = $_POST['firstname'];
$lastname = $_POST['lastname'];
$address = $_POST['address'];
$city = $_POST['city'];
$state = $_POST['state'];
$zipcode = $_POST['zipcode'];
$phone = $_POST['phone'];
$email = $_POST['email'];
// Execute the Statement
$stmt->execute();
}
// Close Statment
$stmt->close();
// Report Successful Entry
$successful = "Client Successfully Created!";
// Define New Client ID
$new_client_id = $link->insert_id;
// FINISH NEW CLIENT PROCESS
goto FinishUpProcess;// CONTINUE FOR SEARCHED PROCESS
SearchReturnProcess:
// Prepared Statement For Database Search
$stmt = $link->prepare("SELECT firstname, lastname, address, city, state, zipcode, phone, email FROM client WHERE client_id=?");
// Bind Client ID into Statement
$stmt->bind_param('s', $client_id);
// Execute the Statement
$stmt->execute();
// Bind Variables to Prepared Statement
$stmt->bind_result($firstname, $lastname, $address, $city, $state, $zipcode, $phone, $email);
//fetch value
$stmt->fetch();
// Close Statment
$stmt->close();
// FINISH SEARCHED PROCESS
goto FinishUpProcess;// CONTINUE FOR UPDATE CLIENT PROCESS
UpdateClientProcess:
// Prepared Statement For Database Search
if ($stmt = $link->prepare("UPDATE client SET firstname=?, lastname=?, address=?, city=?, state=?, zipcode=?, phone=?, email=? WHERE client_id=?")){
// Bind Search Variable
$stmt->bind_param('sssssssss', $firstname, $lastname, $address, $city, $state, $zipcode, $phone, $email, $client_id);
// Define Form Field Input
$firstname = $_POST['firstname'];
$lastname = $_POST['lastname'];
$address = $_POST['address'];
$city = $_POST['city'];
$state = $_POST['state'];
$zipcode = $_POST['zipcode'];
$phone = $_POST['phone'];
$email = $_POST['email'];
$client_id = $_POST['client_id'];
// Execute the Statement
$stmt->execute();
}
// Close Statment
$stmt->close();
// Report Successful Update
$successful = "Client Updated Successfully!";
// FINISH UPDATE PROCESS
goto FinishUpProcess;// CONTINUE FOR FINISHING UP PROCESS
FinishUpProcess:
// Disconnect from Database
mysqli_close($link)
?><!DOCTYPE html>
<html>
<head>
<title>Client Information</title>
<link rel="stylesheet" href="styles.css">
</head>
<body>
<div class="container">
<form id="contact" action="" method="post">
<h4>enter client info below</h4>
<font color="red"><?php echo $successful; ?></font>
<fieldset>
<input name="client_id" value="<?php if (empty($_POST['client_id'])) { echo $new_client_id; } else { echo $_POST['client_id']; } ?>" type="hidden">
</fieldset>
<fieldset>
<font color="red"><?php echo $firstnameErr; ?></font>
<input name="firstname" value="<?php if (isset($_POST['client_id'])) { echo $firstname; } else { echo $_POST['firstname']; } ?>" placeholder="First Name" type="text" tabindex="1" autofocus>
</fieldset>
<fieldset>
<font color="red"><?php echo $lastnameErr; ?></font>
<input name="lastname" value="<?php if (isset($_POST['client_id'])) { echo $lastname; } else { echo $_POST['lastname']; } ?>" placeholder="Last Name" type="text" tabindex="2">
</fieldset>
<fieldset>
<font color="red"><?php echo $emailErr; ?></font>
<input name="email" value="<?php if (isset($_POST['client_id'])) { echo $email; } else { echo $_POST['email']; } ?>" placeholder="Email Address" type="email" tabindex="3">
</fieldset>
<fieldset>
<input name="mailinglist" id="checkbox" type="checkbox" checked>
<label>add to the mailing list</label>
</fieldset>
<fieldset>
<font color="red"><?php echo $phoneErr; ?></font>
<input name="phone" value="<?php if (isset($_POST['client_id'])) { echo $phone; } else { echo $_POST['phone']; } ?>" placeholder="Phone Number" type="tel" tabindex="4">
</fieldset>
<fieldset>
<font color="red"><?php echo $addressErr; ?></font>
<input name="address" value="<?php if (isset($_POST['client_id'])) { echo $address; } else { echo $_POST['address']; } ?>" placeholder="Street Address" type="text" tabindex="5">
</fieldset>
<fieldset>
<font color="red"><?php echo $cityErr; ?></font>
<input name="city" value="<?php if (isset($_POST['client_id'])) { echo $city; } else { echo $_POST['city']; } ?>" placeholder="City" type="text" tabindex="6">
</fieldset>
<fieldset>
<font color="red"><?php echo $stateErr; ?></font>
<input name="state" value="<?php if (isset($_POST['client_id'])) { echo $state; } else { echo $_POST['state']; } ?>" placeholder="State/Province" type="text" tabindex="7">
</fieldset>
<fieldset>
<font color="red"><?php echo $zipcodeErr; ?></font>
<input name="zipcode" value="<?php if (isset($_POST['client_id'])) { echo $zipcode; } else { echo $_POST['zipcode']; } ?>" placeholder="Postal Code" type="text" tabindex="8">
</fieldset>
<fieldset>
<font color="red"><?php echo $countryErr; ?></font>
<input name="country" value="<?php if (isset($_POST['client_id'])) { echo $country; } else { echo $_POST['country']; } ?>" placeholder="Country" type="text" tabindex="9">
</fieldset><fieldset>
<input name="vegan" type="checkbox">
<label>Vegan or Vegitarian</label>
</fieldset>
<fieldset>
<input name="smoker" type="checkbox">
<label>Smoker</label>
</fieldset>
<fieldset>
<textarea name="client_notes" placeholder="general notes" tabindex="10"></textarea>
</fieldset>
<fieldset>
<button name="submit" type="submit" data-submit="...Sending"><?php echo $buttontxt; ?></button>
</fieldset></form>
</div>
</body>
</html>
Решение
Да.
Я не буду проповедовать о ереси и богохульстве, но покажу вам, что большинство ваших GOTO просто неправы.
- UpdateClientProcess. Это довольно странная идея, что вы должны проверять ввод только для создания. Он должен быть всегда одинаковым как для создания, так и для обновления. Так что этот бесполезен и вреден
- FinishUpProcess из процедур проверки. Это ужасно с точки зрения юзабилити. Была старая китайская пытка, когда голова жертвы была зафиксирована под капающим краном. Сначала он был безвредным, он вовремя сводил людей с ума. Итак, вы делаете со своими проверками. Почему бы не проверить ВСЕ поля, а затем сразу сказать пользователю, вместо того, чтобы показывать им ошибки одну за другой?
- FinishUpProcess от сохранения данных. Это нарушает правило протокола HTTP, согласно которому после обработки запроса POST сервер должен выдать заголовок Location, перенаправляющий клиента с помощью метода GET. В противном случае, если клиент обновит страницу, запись будет продублирована.
- Это выглядит грязно. Вы сказали, что. Мне потребовалось немало времени для навигации по вашему коду, чтобы просмотреть его из-за его монотонной структуры. Заполнение кода было изобретено специально. В Python, например, вы принудительный использовать заполнение для различения блоков подчиненного кода.
Правильная структура для этого кода будет выглядеть так:
$errors = [];
if ($_POST) {
if (empty($_POST["firstname"])) {
$errors['firstname'] = "First name is a required field - please make entry below";
}
// and so on
if (!$errors) {
if (empty($_POST['client_id'])) {
// go for insert
} else {
// go for update
}
header("Location: .");
exit;
}
$firstname = htmlspecialchars($_POST['firstname']);
// and so on
}
if (!$errors ) {
if (!empty($_GET['client_id'])) {
// search your data from a GET variable
} else {
// define empty variables
}
}
?>
<html goes here>
Другие решения
Я не уверен, что я даже знал, что goto существовал в PHP. Я использовал (и злоупотреблял) свою долю gotos на протяжении многих лет, но не в последнее время. На исправления:
1 — Многие из ваших gotos (например, SearchReturnProcess) можно заменить вызовами функций. Вместо того, чтобы создавать кусок кода, начинающийся с метки (и использующий goto, чтобы туда попасть), создайте отдельную функцию с тем же именем function SearchReturnProcess() и поместите код там.
2 — Для обработки ошибок используйте if elseif:
if ($_SERVER["REQUEST_METHOD"] == "POST") {
if (empty($_POST["firstname"])) {
$firstnameErr = "First name is a required field - please make entry below";
} elseif (empty($_POST["lastname"])) {
$lastnameErr = "Last name is a required field - please make entry below";
} elseif...
и т.п.
Затем вы можете либо завершить этот набор утверждений else за которым следует блок кода «без ошибок», или вместо множества отдельных ошибок вы можете сделать одну общую переменную ошибки (например, $fieldErr) и после блока есть код вроде if ($fieldErr != '') обрабатывать отображение ошибок и просто отображать ошибки в одном месте, а не рядом с каждым полем.