Wrote a MySQL function - want some feedback and criticism

[Anthony2oo5]

I wrote this function about 2 years ago when I started in PHP, I haven't touched PHP since so im still not an expert, but I would like to know what people thought of it, and if was any good / secure / fast / easy to use ect ect

Looking for some constructive criticism really and improvements and faults.

Thanks

[PHP]<?php

// Database Function
function db($Action) {

$Username = '';
$Password = '';
$Database = '';
$Host = 'localhost';

if ( $Action == 'open' ) {
mysql_connect($ Host, $Username, $Password) or die ( 'Unable to connect to the server. This may be because the server does not exsist, or the username and password given was incorrect. Please check and try again.<br>' );
mysql_select_db ($Database) or die ( 'Unable to select the database. Database does not exsist. Please check and try again.<br>' );
} else if ( $Action == 'close' ) {
mysql_close() or die ( 'Unable to disconnect from the server. Maybe there was no connection to disconnect from?<br>' );
} else {
echo 'The handle that was sent is not valid. Please check and try again.<br>';
}
}
function query($query, $line) {
global $queries, $queries_time, $queries_count;
$queries_count+ +;

// Add the query to the queries list. Dont add a break at the end in case an error occurred
$queries .= 'Line <b>'. $line . '</b> of file <b>' . $_SERVER['PHP_SELF'] . '</b> requested <b>' . $query . '</b>';
// Do the query if it can
$time_overall = round(microtime (), 15);

if (!$result = mysql_query($qu ery)) {
echo 'Error on line <b>'. $line . '</b> of file <b>' . $_SERVER['PHP_SELF'] . '</b> requesting <b>' . $query . '</b><br>';
$queries .= ' <b>ERROR OCCURRED</b><br>';
} else {
$queries .= '<br>';
}

$time2_overall = round(microtime (), 15);
$gen_overall = $time2_overall - $time_overall;
$queries_time = $queries_time + $gen_overall ;

return $result;
}
?>[/PHP]

[Anthony2oo5]

Also IF its is any good, feel free to use it yourself.

[Atli]

Hi.

I have a few tips.

First, and perhaps most important, I would suggest you try a more OOP approach. By that I mean, rather than create several stand-alone functions, try creating a class to handle all Database related operations.
(Check out this article to see how to do that.)

Second, I would upgrade to the MySQLI extension, rather than the old MySQL extension. The latter has been deprecated since MySQL 5, and PHP5, and now that PHP4 has officially reached it's end-of-life, it's time to let the old ways go.

Lastly, and this is just a best-practice sort of thing, I would suggest using more consistent naming conventions. Even tho this may not be an issue now, a function named "db" or "query" could possibly apply to a number of things, which could seriously confuse you (or somebody adopting your code) in the future. Giving then more specific names, like say "DoDatabaseActi on" or "QueryDatab ase" would be much more helpful for people who are trying to understand your code.

It's also always a good idea to be consistent when creating variables and function names, like say always using $lowerCaseCamel Style names for parameters, and the traditional php_underscore_ style() for function names.
Helps make the code more easily understandable.

Hope that helps.

[Anthony2oo5]

Thanks for your comments, I will defiantly be looking into your suggestions.

[Markus]

Hi.

I have a few tips.

First, and perhaps most important, I would suggest you try a more OOP approach. By that I mean, rather than create several stand-alone functions, try creating a class to handle all Database related operations.
(Check out this article to see how to do that.)

Second, I would upgrade to the MySQLI extension, rather than the old MySQL extension. The latter has been deprecated since MySQL 5, and PHP5, and now that PHP4 has officially reached it's end-of-life, it's time to let the old ways go.

Lastly, and this is just a best-practice sort of thing, I would suggest using more consistent naming conventions. Even tho this may not be an issue now, a function named "db" or "query" could possibly apply to a number of things, which could seriously confuse you (or somebody adopting your code) in the future. Giving then more specific names, like say "DoDatabaseActi on" or "QueryDatab ase" would be much more helpful for people who are trying to understand your code.

It's also always a good idea to be consistent when creating variables and function names, like say always using $lowerCaseCamel Style names for parameters, and the traditional php_underscore_ style() for function names.
Helps make the code more easily understandable.

Hope that helps.

MySQL deprecated? Say wha..?

[Atli]

MySQL deprecated? Say wha..?

Technically not, your right, but the MySQL extension was written for MySQL 3, and the MySQLi extension for later MySQL versions:

Although this MySQL extension is compatible with MySQL 4.1.0 and greater, it doesn't support the extra functionality that these versions provide. For that, use the MySQLi extension.

The new MySQLi extension also supports prepared queries (MySQLi_STMT), which are supposedly faster than the normal query functions of either extension. Not to mention safer, as all input data is automatically escaped before execution.