Hello,
I have just finished “build your own DB web sites” (older edition), and I have created something meaningful for my day job. (yipppe).
I was wondering if you would cast your eyes over my hideous code and comment on how I can write more readable, structured and maintainable code.
I know I should be looking at include files and perhaps re-writing certain snippets into functions. But I think the saving will be minimal for the added effort.
Any advice from more experienced members would be greatly appreciated.
file one:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3.org/TR/html4/strict.dtd">
<?php
$dbConn = @mysql_connect('xx','xx','xx');
if (!$dbConn)
{
trigger_error("<p>Unable to connect to database server at this time " . mysql_error() . ".</p>");
}
if (!@mysql_select_db('abtests'))
{
trigger_error("<p>Unable to connect to database at this time " . mysql_error() . ".</p>");
}
?>
<html>
<head>
<title>Conversion experiment(s) master list</title>
<meta http-equiv="Content-type" content="text/html;charset=UTF-8">
</head>
<body>
<h1>holiday autos conversion experiment(s)</h1>
<h2>Filter experiments by:</h2>
<ul>
<?php
$fSQL = @mysql_query("select status, count(*) as count from experiments group by status");
if ($fSQL)
{
while ($fRow = mysql_fetch_array($fSQL))
{
echo "<li>" . "<a href='" . $_SERVER['PHP_SELF'] . "?filter=" . $fRow['status'] . "'>" . $fRow['status'] . " " . "(" . $fRow['count'] . ")" . "</a>" . "</li>\
\ ";
}
echo "<li><a href='" . $_SERVER['PHP_SELF'] . "'>" . "Reset filters" . "</a></li>";
}
else
{
trigger_error("<p>Unable to filter experiments at this time.</p>");
}
?>
</ul>
<table>
<thead>
<tr>
<th>Name</th>
<th>Status</th>
<th colspan="2">User controls</th>
</tr>
</thead>
<tbody>
<?php
$xSelDB = "select id, name, status";
$xFrmDB = " from experiments";
$xWhrDB = " where 1=1";
if (!empty($_GET['filter']) and isset($_GET['filter']))
{
$xSelFilter = $_GET['filter'];
$xWhrDB .= " and status = '$xSelFilter'";
}
$xSelDB = @mysql_query($xSelDB . $xFrmDB . $xWhrDB);
if (!$xSelDB)
{
trigger_error("<p>Unable to select database records at this time " . mysql_error() . "</p>");
}
else
{
while($xRow = mysql_fetch_array($xSelDB))
{
$xId = $xRow['id'];
$xName = $xRow['name'];
$xName = htmlspecialchars($xName);
$xStat = $xRow['status'];
echo "<tr>\
\ ";
echo "<td>" . $xName . "</td>\
\ ";
echo "<td>" . $xStat . "</td>\
\ ";
echo "<td>" . "<a href='experiment.php?edit=$xId'>" . "Edit" . "</a>" . "</td>\
\ ";
echo "<td>" . "<a href='experiment.php?del=$xId'>" . "Delete" . "</a>" . "</td>\
";
echo "</tr>\
";
}
}
?>
</tbody>
</table>
<p><a href="experiment.php?add=1">Add new conversion experiment</a></p>
</body>
</html>
file 2:
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3.org/TR/html4/strict.dtd">
<?php
$dbConn = @mysql_connect('xx','xx','xx');
if (!$dbConn)
{
trigger_error("<p>Unable to connect to database server at this time " . mysql_error() . ".</p>");
}
if (!@mysql_select_db('abtests'))
{
trigger_error("<p>Unable to connect to database at this time " . mysql_error() . ".</p>");
}
?>
<html>
<head>
<?php if (isset($_GET['edit'])): ?>
<title>Edit conversion experiment</title>
<?php elseif(isset($_GET['del'])): ?>
<title>Delete conversion experiment</title>
<?php else: ?>
<title>Add conversion experiment</title>
<?php endif; ?>
<meta http-equiv="Content-type" content="text/html;charset=UTF-8">
</head>
<body>
<?php if (isset($_REQUEST['xName'])): ?>
<?php
if (empty($_REQUEST['xName']) or !isset($_REQUEST['xName']))
{
echo "<h1>Error submitting experiment</h1>";
echo "<p>Please complete the name field.</p>";
echo "<p><a href=" . $_SERVER['PHP_SELF'] . ">Return to form</a></p>";
exit();
}
elseif (isset($_POST['editId']) and isset($_POST['editSub']))
{
$xEditName = ucfirst($_REQUEST['xName']);
$xEditName = htmlspecialchars($xEditName);
$xEditStat = ucfirst($_REQUEST['xStat']);
$xEditid = ucfirst($_REQUEST['editId']);
if (empty($xEditName) or empty($xEditStat) or empty($xEditid)) {
echo "<h1>Error editing experiment</h1>";
echo "<p>You cannot leave any fields empty.</p>";
echo "<p><a href='index.php'>Back to experiment list</a></p>";
exit();
}
else
{
$xEditSQL = "update experiments set name='$xEditName', status='$xEditStat' where id='$xEditid'";
if (@mysql_query($xEditSQL))
{
echo "<h1>Experiment successfully edited</h1>";
echo "<p>$xEditName was edited.</p>";
echo "<p><a href='index.php'>Back to experiment list</a></p>";
}
else {
trigger_error("<p>Unable to edit experiment at this time " . mysql_error() . ".</p>");
}
}
}
else
{
$xSubName = ucfirst($_REQUEST['xName']);
$xSubName = htmlspecialchars($xSubName);
$xSubStat = ucfirst($_REQUEST['xStat']);
$xInsDB = "insert into experiments set name='$xSubName', status='$xSubStat'";
if (@mysql_query($xInsDB))
{
echo "<h1>Experiment successfully added</h1>";
echo "<p>Added: " . $xSubName . "</p>";
echo "<p><a href='index.php'>Back to experiment list</a></p>";
}
else
{
trigger_error("<p>Unable to insert experiment into database at this time " . mysql_error() . ".</p>");
}
}
?>
<?php elseif(isset($_REQUEST['edit'])): ?>
<?php
$xEditId = $_REQUEST['edit'];
$xEditSel = @mysql_query("select name, status from experiments where id='$xEditId'");
$xEditStatOpt = @mysql_query("select distinct status from experiments");
if (!$xEditSel)
{
echo "<p>Unable to edit experiment at this time." . mysql_error() . "</p>";
echo "<p><a href='index.php'>Back to experiment list</a></p>";
exit();
}
$xEditPopFlds = mysql_fetch_array($xEditSel);
$xEditFldsName = $xEditPopFlds['name'];
$xEditFldsStat = $xEditPopFlds['status'];
?>
<h1>Edit conversion experiment</h1>
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
<fieldset>
<legend>Complete all form elements to add an experiment.</legend>
<p><label>Name: <input type="text" name="xName" value="<?php echo $xEditFldsName; ?>" /></label></p>
<p><label>Status:
<select name="xStat">
<?php
while ($statName = mysql_fetch_array($xEditStatOpt))
{
$optStat = $statName['status'];
if ($optStat == $xEditFldsStat)
{
echo "<option value='$optStat' selected='selected'>$optStat</option>\
\ ";
}
else
{
echo "<option value='$optStat'>$optStat</option>\
\ ";
}
}
?>
</select>
</label></p>
<input type="hidden" name="editSub" value="true" />
<input type="hidden" name="editId" value="<?php echo $xEditId; ?>" />
<p><button>Edit experiment</button></p>
</fieldset>
</form>
<p><a href="index.php">Back to experiment list</a></p>
<?php elseif(isset($_REQUEST['del'])): ?>
<h1>Are you sure you would like to delete?</h1>
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>?delConfirm=1">
<input type="hidden" name="delSub" value="true" />
<input type="hidden" name="delId" value="<?php echo $_REQUEST['del']; ?>" />
<p><button>Delete experiment</button></p>
</form>
<p><a href="index.php">Back to experiment list</a></p>
<?php elseif (isset($_GET['delConfirm']) and isset($_POST['delSub'])): ?>
<?php
$xDelId = $_POST['delId'];
$xDelSQL = "delete from experiments where id='$xDelId'";
if (@mysql_query($xDelSQL))
{
echo "<h1>Experiment successfully deleted</h1>";
}
else
{
trigger_error("<p>Unable to delete experiment at this time " . mysql_error() . ".</p>");
}
?>
<p><a href="index.php">Back to experiment list</a></p>
<?php else: ?>
<h1>Complete the form to add a conversion experiment to the list</h1>
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
<fieldset>
<legend>Complete all form elements to add an experiment.</legend>
<p><label>Name: <input type="text" name="xName" /></label></p>
<p><label>Status:
<select name="xStat">
<option value="0">Choose status</option>
<option value="1">------</option>
<option value="Queue">Queue</option>
<option value="Development">Development</option>
<option value="Underway">Underway</option>
<option value="Complete">Complete</option>
</select>
</label></p>
<p><button>Submit experiment</button></p>
</fieldset>
</form>
<p><a href="index.php">Back to experiment list</a></p>
<?php endif; ?>
</body>
</html>