PDA

View Full Version : critique



jeaux
08-21-2008, 01:25 PM
I finally got this working, and all it does is populate a drop-down, but I had a few questions.

1. Where should I put my validation?
2. Should I put the form action script on this page?
3. I know there's more than one way to skin a cat(not that I would ever do that), are there some built-in functions of php that would have made this easier?
4. Separating code and markup does make it easier to read in Dreamweaver and PhpED, but is that really the way the pros do it.



<?php
//deleted connection data
$conn = mysql_connect($dbhost, $dbuser, $dbpass)
or die('Error connecting to MySQL.');

mysql_select_db($dbname)
or die('Error selecting database.');
//following are all fields remember to change to *
$query='SELECT MenuCategoryID, MenuCategoryName FROM universitymenucategory';
$result=mysql_query($query);
?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Add menu item</title>
</head>

<body>
<form action="../ManagerSection/PHP_Scripts/AddMenuItem.php"
method="post">
<fieldset>
<legend>Add menu item</legend>
<ol>
<li><label for="form-ItemName">Item name:</label> <input type="text"
name="name" id="form-name"></li>
<li><label for="form-ItemPrice">Price: $</label><input type="text" name="price" id="form-price"></li>
<li><label for="form-ItemDescription">Description:</label><textarea name="Description" rows="4" cols="60" maxlength="300">(300 characters max)</textarea></li>
<li><label for="form-Category">What category does this menu item belong?<label><br />
<select name="MenuCategory" id="form-Category">
<?php
while ($row = mysql_fetch_array($result)) {
?>
<option value="
<?php
echo $row['MenuCategoryID']
?>
">
<?php
echo $row['MenuCategoryName'];
?>
</option>
<?php
}
?>
</select></label></li>
</ol>
<input type="submit" value="Add item">
</fieldset>
</form>
</body>
</html>

5. If I wanted this to work in php5 is this the only alteration I would need?


$conn = mysql_connect($dbhost, $dbuser, $dbpass)
or die('Error connecting to MySQL.');

mysql_select_db($dbname)
or die('Error selecting database.');
to

$conn = mysqli_connect($dbhost, $dbuser, $dbpass, $dbname)
or die('Error connecting to MySQL.');


Many thanks,
Joe

techietim
08-21-2008, 01:30 PM
1. Where should I put my validation?
2. Should I put the form action script on this page?
3. I know there's more than one way to skin a cat(not that I would ever do that), are there some built-in functions of php that would have made this easier?
4. Separating code and markup does make it easier to read in Dreamweaver and PhpED, but is that really the way the pros do it.

1) You should have PHP validation on the page you're submitting it to.
3) Next
4) Yes, the "pros" use MVC frameworks which separates markup from scripting. (Google PHP MVC Framework and you'll find tons)

Twey
08-21-2008, 03:29 PM
5. If I wanted this to work in php5 is this the only alteration I would need?You don't even need that. mysqli is not mandatory, it just makes things easier.
4. Separating code and markup does make it easier to read in Dreamweaver and PhpED, but is that really the way the pros do it.Yes — as I said in your previous thread. It makes it easier to read, full stop -- and it also means that you can edit one without having to fiddle with the other.
3. I know there's more than one way to skin a cat(not that I would ever do that), are there some built-in functions of php that would have made this easier?This is a very simple page, there aren't really many ways to make it simpler (no, next (http://www.php.net/next)() would not help, since we are not dealing with an array — if we were, I would have recommended the use of foreach (http://www.php.net/foreach) over next()). As part of a larger site, or if you're doing this kind of thing a lot, one of the frameworks mentioned in the above post might help you. They include things like custom URL-routing routines, proper template engines to separate code from output even more, and ORMs to make working with databases easier and more convenient.
2. Should I put the form action script on this page?Probably not. The 'PHP way' is 'one page per file'. I would advise creating a separate page.
1. Where should I put my validation?On that separate page.

Indent properly! Nothing ruins code readability quite so much as lack of indentation. I was about to critique this further, but then recognised your username and realised that almost all of the points I would have made were all in your other thread (http://dynamicdrive.com/forums/showthread.php?t=35600) already — and that you've completely ignored several of them, even going so far as to change back some things I modified (HTML to XHTML was the one that struck me first).

jeaux
08-21-2008, 05:35 PM
I was about to critique this further, but then recognised your username and realised that almost all of the points I would have made were all in your other thread already — and that you've completely ignored several of them, even going so far as to change back some things I modified (HTML to XHTML was the one that struck me first).

This is a different page than the one that you worked on, so nothing was changed back. The analysis that you gave me was to lose the transitional because it was outdated(10 years or something). I did start using single quotes, separated code and output, lost the MYSQL_ASSOC, but did forget about the wasted variable here:


$query='SELECT MenuCategoryID, MenuCategoryName FROM universitymenucategory';


Our last interaction was a lot for a noob to digest. Many apologies for ruffling your feathers.

OK I saw the xhtml.

Twey
08-21-2008, 05:50 PM
Ah, sorry, I presumed it was a modification.

One of the more basic uses of PHP is to keep the layout and content in separate files so that you don't have to include your layout code in page.

jeaux
08-21-2008, 08:22 PM
The other file is gold and is in the vault. I do still need to revisit it to change the <dl>s to a table markup, but I wanted to move on with my project. I suppose a simple cms was probably not the brightest idea to start my php understanding, but I had to start somewhere.

Anyway thanks,
Joe

Twey
08-21-2008, 11:22 PM
If it's a CMS you're doing, a framework really will make your life a heck of a lot easier. Have a look at CakePHP (http://cakephp.org/).

jeaux
08-22-2008, 01:28 AM
I know but that's not the point of my project. In the past I did a site with dotnetnuke and never could figure out what the heck was going on. I got the site to do what I wanted it to do, but only by studing the cms itself not VB. My understand of VB remained the same through the entire project. I figured that if I built this from the ground up that I would have a deeper understanding not of the cms but of php. I actually did download cake and was lost from the start.

Thanks for the advice, but I'm going to try this one book at a time, one page at a time, one thread at a time.

Twey
08-22-2008, 10:23 PM
Vaguely relevant: http://www.catonmat.net/download/model_view_controller_song.mp3

mburt
08-22-2008, 10:28 PM
That's just about the cheesiest thing i've heard in a long time... haha.