PDO connection function

Hi,

I have constructed my PDO connection function as below.
1. How does it look? Do you see any problem? Any comment to make it better?
2. I don't use function to close the connection (like mysql_close). I read somewhere it is better for PDO connection performance, is that ok?

Thank you


function dbconnect()
{

global  $dbpdo;

	if ($_SESSION['dbchooseses']==1) {
	$db_username='dbuser';
	$db_name='dbname';
	$db_password = 'psw';
	$db_host ='localhost';
	}else if ($_SESSION['dbchooseses']==2) {
	$db_username='dbuser2';
	$db_name='dbname2';
	$db_password = 'psw2';
	$db_host ='localhost';
	}
	
	try {
    $dbpdo = new PDO("mysql:host=$db_host;dbname=$db_name", $db_username, $db_password, array(PDO::ATTR_PERSISTENT => true));
	$dbpdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
	$dbpdo->setAttribute(PDO::ATTR_TIMEOUT, 30);
	$dbpdo->query("SET NAMES utf8");  
	}
	
	catch(PDOException $e)
    {
	$e->getMessage();
	die();
	}
	
	
}	

function selectsql($sql) {
	global  $dbpdo;
	$r = $dbpdo->query($sql)->fetchAll();
	if ($r[0]) return $r[0]; 
	else return Array();
}

function selectsqln($sql) {
	global  $dbpdo;
	$r = $dbpdo->query($sql)->fetchAll();
	return $r;
}
function selectsqlp($sql,$arr) {

	global  $dbpdo;
	try{
	$q = $dbpdo->prepare($sql);
	$q-> execute($arr);
	}catch (PDOException $e) {
	$e->getMessage();
	$r = $q ->fetchAll();
	return $r[0];
}
function selectsqlpn($sql,$arr) {

	global  $dbpdo;
	try{
	$q = $dbpdo->prepare($sql);
	$q-> execute($arr);
	}catch (PDOException $e) {
	$e->getMessage();
	$r = $q ->fetchAll();
	return $r;
}
function insertsql($sql) {
	
	checktime($sql);
	global  $dbpdo;
	try{
	$dbpdo->query($sql);
	}catch (PDOException $e) {
	$e->getMessage();
	$r = $dbpdo->lastInsertId();
	return $r;
}
function insertsqlp($sql,$arr) {
	checktime($sql);
	global  $dbpdo;
	$q = $dbpdo->prepare($sql);
	try{
	$q-> execute($arr);
	}catch (PDOException $e) {
	$e->getMessage();
	$r = $dbpdo->lastInsertId();
	return $r;
}
function updatesql($sql) {
	
	global  $dbpdo;
	try{
	$dbpdo->query($sql);
	}catch (PDOException $e) {
	$e->getMessage();
	
}
function updatesqlp($sql,$arr) {
	
	global  $dbpdo;
	$q = $dbpdo->prepare($sql);
	try{
	$q-> execute($arr);
	}catch (PDOException $e) {
	$e->getMessage();
	return $q;
}

Open in new window

LVL 1
myyisAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

GaryCommented:
Can you give some examples of your $sql and $arr - just to be sure you are preparing everything correctly.
For example here
function insertsql($sql) {

You don't have any params passed so I assume you are building the sql directly which defeats the purpose of PDO make data safe queries.
0
myyisAuthor Commented:
When the parameters are created by the code, I use this.
This is the case I am sure that the data is safe

$sql="INSERT INTO  LOGIN_HISTORY (LOGIN_ID,SESSION_ID,IP,LOGIN_DATE_TIME,ORID) VALUES ('$userId','$session_id','$ipset','$login_date_time','$oridtry')";
insertsql($sql);

When I need to check the data I use this

$sql = "insert into CONTACT (NAME,TYPE,ORID,USERID,DATETIME,USERIDEDIT,DATETIMEEDIT,PARENTCONTACTNAME,PARENTCONTACTID)  values (?,'P','$orid','$uid','$date','$uid','$date','$cname','$cid')";
                  
$cid=insertsqlp($sql,array($searchStr));
0
GaryCommented:
That is not how you should be using PDO, part of its purpose is to make sanitizing inserted data easy.

Example process would be
Prepare the statement - :user_id is the variable we use in the execute
$stmt = $conn->prepare('select * from table where id=:user_id)');

In the execute assign the user_id variable to its value
$stmt->execute(array(':user_id'=>$user_id));

So while your functions are ok you should be passing the sql statement in a prepared way and then the array of values to use in the statement.

Another benefit of PDO is that when you are using prepared statements you only need to declare them once and then you only have to use the execute line each time you need to make the same query

You say the data is safe, but it would be better to get into the practise of doing it the right way.
0
10 Tips to Protect Your Business from Ransomware

Did you know that ransomware is the most widespread, destructive malware in the world today? It accounts for 39% of all security breaches, with ransomware gangsters projected to make $11.5B in profits from online extortion by 2019.

myyisAuthor Commented:
In the example below $date is generated by php server , so I don't have to sanitize it.
Is this right? When I am preparing the statement, do I have to put all the parameters to array values?




function insertsqlp($sql,$arr) {
      
      global  $dbpdo;
      $q = $dbpdo->prepare($sql);
      try{
      $q-> execute($arr);
      }catch (PDOException $e) {
      $e->getMessage();
      $r = $dbpdo->lastInsertId();
      return $r;
}


$sql = "insert into CONTACT (NAME,TYPE,ORID,USERID,DATETIME,USERIDEDIT,DATETIMEEDIT,PARENTCONTACTNAME,PARENTCONTACTID)  values (?,'P','$orid','$uid','$date','$uid','$date','$cname','$cid')";
                 
$cid=insertsqlp($sql,array($searchStr));
0
GaryCommented:
If the vars are generated by you then they don't need sanitizing.
No you don't have to put everything in the array, just the values you want to keep separate.
PDO works on two layers...
It gives MySQL the insert/update etc statement then it gives MySQL the data. The data is separated from the query so no malicious code can be injected etc.
You are complicating your code with all the functions when you could just prepare all the statements you are going to use in one block

$stmtSelect = $conn->prepare('select * from table where id=:user_id)');
$stmtInsert = $conn->prepare('insert into table set (id) values (:user_id)');
...
and so on.

then wherever you need to run any of those statements you can simple do your execute.
$stmtSelect->execute(array(':user_id'=>$user_id));

This is especially a better way of doing it if you are going to be doing multiple inserts because you have the sql statement already prepared and do not need to do it again.
Another factor is if you need to insert a 100 rows for example you can still use the same prepared statement and build your array of values to insert and execute it in one go.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Ray PaseurCommented:
This article has examples that map MySQL, MySQLi and PDO across equivalent functions.
http://www.experts-exchange.com/Web_Development/Web_Languages-Standards/PHP/PHP_Databases/A_11177-PHP-MySQL-Deprecated-as-of-PHP-5-5-0.html

This code snippet shows a Singleton design for a data base connection.  Using the information in the article, it would be possible to replace the extension with the PDO extension.

<?php // RAY_database_singleton.php
error_reporting(E_ALL);


// SINGLETON DATA BASE CONNECTION CLASS
class Database
{
    // CLASS PROPERTIES ARE ALL PRIVATE
    private static $connection;
    private static $instance;

    // CONNECTION VALUES (GET THESE FROM YOUR HOSTING COMPANY
    const DB_HOST = '?';
    const DB_USER = '?';
    const DB_PASS = '?';
    const DB_NAME = '?';

    // NULLIFY THE CLONE
    final private function __clone() {}

    // OUR ONLY PUBLIC METHOD RETURNS THE CONNECTION
    public static function getConnection()
    {
        if (!self::$instance) self::$instance = new self();
        return self::$connection;
    }

    // CONSTRUCTOR RETURNS THE CONNECTION
    private function __construct()
    {
        self::$connection
        = new mysqli
        ( self::DB_HOST
        , self::DB_USER
        , self::DB_PASS
        , self::DB_NAME
        )
        ;
        if (self::$connection->connect_error)
        {
            trigger_error(self::$connection->connect_error, E_USER_ERROR);
        }
    }
}

$mysql1 = database::getConnection();
$mysql2 = database::getConnection();


// PROVE THAT THESE ARE THE SAME OBJECT http://php.net/manual/en/language.oop5.object-comparison.php
if ($mysql1 === $mysql2) echo 'EQUAL';

// SHOW THE OBJECT
var_dump($mysql1);

Open in new window

0
myyisAuthor Commented:
Ok one last question

I don't use function to close the connection (like mysql_close). I read somewhere it is better for PDO connection performance, is that ok?
0
GaryCommented:
The connection would be closed when the page finishes executing anyway, so no need to manually close it.
Cannot see how it would affect performance manually closing it unless you were reopening it again.
0
Ray PaseurCommented:
@myyis: Please, please, please buy this book and don't write any PHP code that has any economic value until you've finished working your way through it.  I've just re-read this question and it seems to me that you have some giant holes in your understanding of computer science and PHP.  That is normal and to be expected when you're just starting out, and the Welling/Thompson book can help you fill in the blanks with something that will not get you fired (or worse, sued!)
0
myyisAuthor Commented:
Thank you Ray I will buy the book, it looks very useful.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
PHP

From novice to tech pro — start learning today.