Code Review PLEASE !!!
| From: | Matthew Oatham | Date: | Mon, 05 Apr 2004 22:28:37 +0000 |
| Subject: | Code Review PLEASE !!! | ||
| Groups: | php.general | ||
| Request: | Send a blank email to php-general+get-182412@lists.php.net to get a copy of this message | ||
Hi,
I am a newbie PHP programmer, I have some code that works but I want some tips on how I an Improve
my code, i.e. should I be doing my updates / deletes on same php page as the display page, am I
using transactions correctly, am I capturing SQL errors correctly am I handling form data as
efficient as possible?
My code displays some information from a database and gives users the chance to delete or edit any
field and is as follows:
<?
include ("../db.php");
$acton = $_POST['action'];
if ($action == "update") {
if (isset($_POST['delete'])) {
$deleteList = join(', ', $_POST['delete']);
}
//Enter info into the database
mysql_query("begin");
foreach ($_POST['fleet_id'] as $key => $value) {
$fleetCode = $_POST['fleet_code'][$key];
$historyUrl = $_POST['history_url'][$key];
$downloadUrl = $_POST['download_url'][$key];
mysql_query("UPDATE imp_fleet SET fleet_code = '$fleetCode', history_url =
'$historyUrl', download_url = '$downloadUrl' WHERE fleet_id = $value") or
die (mysql_error());
}
if ($deleteList) {
mysql_query("DELETE FROM imp_fleet WHERE fleet_id IN($deleteList)") or die
(mysql_error());
}
if (mysql_error()) {
echo ("There has been an error with your edit / delete request. Please contact the
webmaster");
mysql_query("rollback");
} else {
mysql_query("commit");
}
}
?>
<html>
<head>
<title></title>
</head>
<body>
<form name="edit" method="post">
<h1>Edit / Delete Fleet</h1>
<table>
<tr>
<td>Fleet Code</td>
<td>Download URL</td>
<td>History URL</td>
<td>Delete</td>
</tr>
<?
$sql = mysql_query("SELECT fleet_id, fleet_code, download_url, history_url FROM
imp_fleet");
if (mysql_num_rows($sql) > 0) {
while ($row = mysql_fetch_array($sql)) {
?>
<tr>
<td><input type="text" name="fleet_code[]"
value="<?=$row['fleet_code']?>"><input type="hidden"
name="fleet_id[]" value="<?=$row['fleet_id']?>"></td>
<td><input type="text" name="download_url[]"
value="<?=$row['download_url']?>"></td>
<td><input type="text" name="history_url[]"
value="<?=$row['history_url']?>"></td>
<td><input type="checkbox" name="delete[]"
value="<?=$row['fleet_id']?>"></td>
</tr>
<?
}
}
?>
<tr>
<td colsapn="4">
<table>
<tr>
<td><input type="hidden" name="action"
value="update"><input type="reset" value="cancel"></td>
<td colspan="2"><input type="submit"
value="submit"></td>
</tr>
</table>
</td>
</tr>
</table>
</form>
</body>
</html>
Thanks for your time and feedback.
Matt