Page 1 of 1

Small Scrip effeciency

Posted: Fri Sep 19, 2008 10:52 am
by Weasel5-12
Sorry guys, hah forgot 2 explain myself.

I've written the following as pt2 of an processing form.. i was wondering if any new eyes could see how the effeciency could be better.
I'm always up for good clean code, so please dont hesitate to call it fugly, as long as u can explain why:D

Code: Select all

<?php 
/* Program: uploader.php
* Description: Uploads data takem from addGlassInfo.php
*/
 
[color=#008000]include[/color]([color=#FF0000]"misc.inc"[/color]);
 
$connection = [color=#0000FF]mysql_connect[/color]($host, $user, $password) [color=#0000FF]or die[/color] ([color=#FF0000]"Couldn't connect to server"[/color]);
$db = [color=#0000FF]mysql_select_db($database, $connection)[color=#0000FF]or die[/color] ([color=#FF0000]"Couldn't connect to database"[/color]);
 
// Variable Declaration
$name = [color=#0000FF]$_POST[/color]['name'];
$description = [color=#0000FF]$_POST[/color]['description'];
$instructions = [color=#0000FF]$_POST[/color]['instructions'];
$glass_type = [color=#0000FF]$_POST[/color]['glass_type'];
$pic = [color=#0000FF]$_POST[/color]['pic'];
$ingredientNum = [color=#0000FF]$_POST[/color]['numOfIngredients'];
 
$process = [color=#FF0000]"addDrinkProcess_2.php"[/color];
 
/* Checks if drink already exists in drinklist table. If not, add it to table */
$query = [color=#FF0000]"SELECT drinkName FROM drinklist WHERE drinkName='$name'"[/color];
 
$result = [color=#0000FF]mysql_query[/color]($query)[color=#0000FF]or die[/color] ([color=#0000FF]mysql_error()[/color]);
 
$ntype = [color=#0000FF]mysql_num_rows[/color]($result);
[color=#008000]if[/color]($ntype != 0){
    [color=#0000FF]echo[/color] [color=#FF0000]"<span class='style2'><h4>NOTE: The following already exist in the database:</h4></span>"[/color];
    [color=#0000FF]echo[/color] [color=#FF0000]"<hr>"[/color];
    [color=#008000]include[/color]([color=#FF0000]"dataOutputTable_1.inc"[/color]);
 
}[color=#008000]else[/color]{
    $query = [color=#FF0000]"INSERT INTO drinklist (drinkID, drinkName, drinkInstructions, drinkDescription, drinkPicture, glassID) VALUES ('NULL' , '$name', '$instructions', '$description', '$pic', '$glass_type')"[/color];
    $result = [color=#0000FF]mysql_query[/color]($query)[color=#0000FF]or die[/color] ([color=#0000FF]mysql_error()[/color]);
    [color=#0000FF]echo[/color] [color=#FF0000]"<h2>The following Have been added to the database:</h2>"[/color];
 
    // Brings in external table file for output.
    [color=#008000]include[/color]([color=#FF0000]"dataOutputTable_1.inc"[/color]);
    
    [color=#0000FF]echo[/color] [color=#FF0000]"<br><hr><br>"[/color];
    
    // .......
    //[color=#0000FF]echo[/color] [color=#FF0000]"<form id='form1' name='form1' method='post' action=''>"[/color];
    [color=#008000]for[/color]($x = 0; $x < $ingredientNum; $x++){
    $result_1 = [color=#0000FF]mysql_query[/color]([color=#FF0000]"SELECT * FROM ingredientlist"[/color]) [color=#0000FF]or die[/color] ([color=#0000FF]mysql_error()[/color]);
    $result_2 = [color=#0000FF]mysql_query[/color]([color=#FF0000]"SELECT * FROM setamount"[/color]) [color=#0000FF]or die[/color] ([color=#0000FF]mysql_error()[/color]);
    [color=#0000FF]echo[/color] [color=#FF0000]"Ingredient: "[/color];
    [color=#0000FF]echo[/color] [color=#FF0000]"<select name='ingredient[]'><br>\n"[/color];
    
    [color=#008000]while [/color]($row1 = [color=#0000FF]mysql_fetch_array[/color]($result_1)){
        extract($row1);
        [color=#0000FF]echo[/color] [color=#FF0000]"<option value='"[/color].$ingredientID.[color=#FF0000]"'>"[/color].$ingredientName.[color=#FF0000]"<br>\n"[/color];
    }
    
    [color=#0000FF]echo[/color] [color=#FF0000]"</select>"[/color];
    [color=#0000FF]echo[/color] [color=#FF0000]"Amount:\n"[/color];
    [color=#0000FF]echo[/color] [color=#FF0000]"<select name='amount[]'><br>\n;"[/color];
    
    [color=#008000]while[/color]($row2 = [color=#0000FF]mysql_fetch_array[/color]($result_2)){
        extract($row2);
        [color=#0000FF]echo[/color] [color=#FF0000]"<option value='"[/color].$amount.[color=#FF0000]"'>"[/color].$amount.[color=#FF0000]"<br>\n"[/color];
        }
        [color=#0000FF]echo[/color] [color=#FF0000]"</select><br />"[/color];
    
    }
    
    [color=#0000FF]echo[/color] [color=#FF0000]"</form>"[/color];
}               
?>
Cheers guys/gals

Re: Small Scrip effeciency

Posted: Fri Sep 19, 2008 11:36 am
by Maugrim_The_Reaper
In your variable description stage you should apply some input filtering to ensure the data you get is what you expect it to be. You should also escape the verified data with something like mysql_real_escape_string() before putting it into an SQL statement. On anything you output to HTML you should also escape using htmlspecialchars().

I'd read up on basic PHP security - it's not as complex as people think and it's just a set of habits you need to adhere to for all code you write.

Re: Small Scrip effeciency

Posted: Fri Sep 19, 2008 1:02 pm
by Weasel5-12
thx alot dude. appreciated