Page 1 of 2
How bad is my PHP scripting?
Posted: Mon Aug 26, 2002 4:04 am
by Todd
Ok,
I am pretty new at this PHP thing.
I was wondering if someone could look at a script that I made and tell me how poorly written it is and give me suggestions on what I could or should have done to make it better or more simple.
Here is the link to my script:
myscript.txt
Thank you!
Todd
Posted: Mon Aug 26, 2002 4:42 am
by Xelmepa
You've have made quite a few mistakes...
First of all, MySQL:
Connection to MySQL should be done like this:
Code: Select all
$connection = mysql_connect("localhost","username","password");
This way you can close the MySQL connection when you won't need it anymore. Also, you have opened the connection in the beggining of the script, and actually used it at the mid-end, which just leaves the connection open for some time. You should make the connection when you need it...
Also, you have done around a hundred print(); functions, that you dont need to. You can as well just close PHP when you really don't need it (just use "?>"), throw in all the HTML, and start php again (using "<?"), instead of calling print(); all these times...
These are the major faults I can see.
Posted: Mon Aug 26, 2002 8:54 am
by volka
mysql_connect("localhost","username","password");
mysql_select_db("database");
$sql = mysql_query("select * from trade_skills ORDER BY '$field' $order");
your select-string uses $field and $order. if register_globals is Off (in newer versions of php) you have to get the values from $_GET['field'] and $_GET['order'];
tip:
use .... or die(mysql_error()); expressions while developing i.e.
mysql_select_db("database") or die(mysql_error());
this will display the reason why select_db failed (if) and abort the script.
If it complains about a query-string, assign that query to a var. first and print it out, too.
i.e. $result = mysql_query($query) or die ($query . ' -> '. mysql_error());
sometimes you may want to get some "debug-informations" but do not want to interfere with the html-output-layout. Then it's time to use html comments

i.e
Code: Select all
while($row = mysql_fetch_row($result))
{
$dbgOut = var_dump($row);
$dbgOut = htmlentities($dbgOut); // not always the best solution
print('<!-- ' . $dbgOut . ' -->');
...
Thnak you for you input!
Posted: Mon Aug 26, 2002 11:59 am
by Todd
It is a real help!
thank you!
Todd
Posted: Mon Aug 26, 2002 2:30 pm
by Takuma
Code: Select all
Blah Blah Blah ("select * from trade_skills ORDER BY '$field' $order");
I don't think you need '$field', I think it can be just $field without 's.
Posted: Wed Aug 28, 2002 9:46 am
by gotDNS
NEWS FLASH: Forget PRINT! now use ECHO!
:: shrug :: ... it looks kooler
Posted: Wed Aug 28, 2002 9:59 am
by volka
I like prefix-function-notation

print() forever
Posted: Wed Aug 28, 2002 4:24 pm
by Takuma
What's the difference between print & echo?
Posted: Wed Aug 28, 2002 5:02 pm
by hob_goblin
Pretty much: Nothing.
Posted: Wed Aug 28, 2002 5:15 pm
by codewarrior
huh?

Still learning

Posted: Wed Aug 28, 2002 6:21 pm
by volka
Posted: Wed Aug 28, 2002 6:59 pm
by m3mn0n
What is the difference between echo and print?
Which is faster, echo or print?
Jun 8th, 1999 09:00
Nathan Wallace
Rasmus Lerdorf
There is a difference between the two, but speed-wise it
should be irrelevant which one you use. print() behaves
like a function in that you can do:
And $ret will be 1
That means that print can be used as part of a more complex
expression where echo cannot. print is also part of the
precedence table which it needs to be if it is to be used
within a complex expression. It is just about at the bottom
of the precendence list though. Only "," AND, OR and XOR
are lower.
echo is marginally faster since it doesn't set a return
value if you really want to get down to the nitty gritty.
If the grammar is:
Code: Select all
echo expression ї, expressionї, expression] ... ]
Then
is not valid. ( expression ) reduces to just an expression
so this would be valid:
but you would simply write this as:
if you wanted to use two expression. Putting the brackets
in there serves no purpose since there is no operator
precendence issue with a single expression like that.
Posted: Wed Aug 28, 2002 7:08 pm
by volka
of course, one may post the complete article here, too

Posted: Wed Aug 28, 2002 8:25 pm
by phice
Also, ya'll forgot about his if...else... coding..
Code: Select all
if($order == "DESC")
$order = "ASC";
else
$order = "DESC";
it should be...
Code: Select all
if ($order == "DESC") {
unset($order);
$order = "ASC";
} else {
if (isset($order)) { unset($order) }
$order = "DESC";
}
Posted: Wed Aug 28, 2002 8:43 pm
by hob_goblin
there is no need to unset them...