Code:
<?php
$dbhost = 'localhost';
$dbuser = 'user';
$dbpass = 'password';
$dbname = 'Restaurant';
$conn = mysql_connect($dbhost, $dbuser, $dbpass)
or die('Error connecting to MySQL.');
mysql_select_db($dbname)
or die('Error selecting database.');
$rs_menucategories = mysql_query('SELECT MenuCategoryID, MenuCategoryName FROM universitymenucategory');
$q_menuitems = 'SELECT MenuCategoryID, ItemName, ItemCost, ItemDescription'
. 'FROM universitymenu'
. 'WHERE MenuCategoryID = %d';
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title>Menu</title>
<link href="../CSSFolder/menu.css" rel="stylesheet" type="text/css">
</head>
<body>
<div id="container">
<div class="horbar">
<a href="../index.htm">Home</a>
<a href="menu.htm">Menu</a>
<a href="wine.htm">Wine</a>
<a href="specials.htm">Specials</a>
<a href="events.htm">Events</a>
<a href="map.htm">Map</a>
<a href="contact.htm">Contact</a>
</div>
<div id="menu-container">
<div class="gutter">
<?php while ($crow = mysql_fetch_array($rs_menucategories)) { ?>
<h2 class="menu-category">
<?php echo $crow['MenuCategoryName']; ?>
</h2>
<dl>
<?php $rs_menuitems = mysql_query(sprintf($q_menuitems, $crow['MenuCategoryID']));
while ($irow = mysql_fetch_array($rs_menuitems)) { ?>
<dt>
<?php echo $irow['ItemName']; ?>
</dt>
<dd class="price">
<?php echo $irow['ItemCost']; ?>
</dd>
<dd class="ingredients">
<?php echo $irow['ItemDescription']; ?>
</dd>
<?php } ?>
</dl>
<?php } ?>
</div> <!-- end gutter -->
</div> <!-- end menu-container -->
</div> <!-- end container -->
</body>
</html>
I'm not a programmer, I've only read a couple of books.
Then, from the top:
Code:
$querymenucategory = "SELECT MenuCategoryID, MenuCategoryName FROM universitymenucategory";
- You only ever use this variable once and it isn't necessary for code organisation. There's no point in it being there, it's a waste of memory.
- If you're not going to interpolate variables into the string, don't use double quotes -- there's no need and it's slower to parse.
Code:
$querymenuitem = "SELECT MenuCategoryID, ItemName, ItemCost, ItemDescription FROM universitymenu";
$resultmenuitem = mysql_query($querymenuitem);
- You can't execute this query yet, as you don't have the necessary ID.
Code:
echo "<!DOCTYPE html PUBLIC
- Woah woah woah! Code and output should be kept as separate as possible. This means that if you find yourself echoing a big chunk of markup, you should break out of PHP parsing mode. If you find yourself echoing a medium chunk of markup, you should break out of PHP parsing mode. If you find yourself echoing a small chunk of markup, chances are you should break out of PHP parsing mode.
Code:
<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">
- The Transitional line of DOCTYPEs were designed to ease the move from HTML3, some decade ago now. They're outdated. Only Strict DOCTYPEs should be used on the Web of today.
- XHTML, however, is not currently supported by IE, which means that you probably shouldn't use it for pages where you don't control the browser everyone is going to be using to access your site (i.e. every internet page and many intranet ones). You can get some more info at http://www.webdevout.net/articles/beware-of-xhtml.
- We're beginning to see one of the penalties of echoing a whole page of markup as one big string: having to escape every single special character.
Code:
while($row = mysql_fetch_array($resultmenucategory, MYSQL_ASSOC))
- You don't need to specify MYSQL_ASSOC, the default is MYSQL_BOTH and there's negligible to no overhead to providing the numerical keys as well.
Code:
echo "<h2 class =\"menu-category\">{$row['MenuCategoryName']}</h2>\n" .
"<dl>\n";
- I'm pretty sure you don't want an <h2> and a <dl>. I think you'd probably be better off with a <table> here -- this is what tables are for. I've left this alone in my revision of your code because I don't know what your stylesheet is doing, but that's something you should consider changing.
Code:
while($row = mysql_fetch_array($resultmenuitem, MYSQL_ASSOC))
- Oops, careful! You've overwritten your original $row variable. That's not going to give you what you think it is on the next pass.
- This is about where we should be doing that original mysql_query(): we've got our ID now and can fit it into the query string so we only get the rows we need.
Good luck with your programming hobby/career of choice!
Bookmarks