patternphpMinor
MySQL connection class
Viewed 0 times
mysqlclassconnection
Problem
I am trying to learn OOP using PHP5 and I wrote a simple MySQL connection class. Please take a look and give me some feedback on better practices, critical errors, and any other feedback you can provide.
Note: in the
Note: in the
verifyDatabaseConnection method, I used an @ symbol to curb the error I was receiving.user = $user;
$this->pass = $pass;
$this->data = $data;
$this->host = $host;
$this->verifyNullFields();
}
private function verifyNullFields()
{
if($this->user == NULL)
{
print('mysql error : username is null');
}
if($this->data == NULL)
{
print('mysql error : database name is null');
}
else if($this->host == NULL)
{
print('mysql error : host name is null');
}
else
{
$this->verifyDatabaseConnection();
}
}
private function verifyDatabaseConnection()
{
$link = @mysql_connect($this->host,$this->user,$this->pass);
if(!$link)
{
die('mysql error : databse connection issue');
}
else
{
$this->verifyDatabaseExist();
}
}
private function verifyDatabaseExist()
{
$db = mysql_select_db($this->data);
if(!$db)
{
die('mysql error : database selection issue');
}
}
}
?>
Solution
-
Don't print anything or call
-
Get rid of chained method calls. Don't call
-
Rename
-
At your opinion: you may replace
-
Also, when you will need to create similar class for PostgreSQL database (or abother DB) you should extract base class
Don't print anything or call
die() from this class because it may breaks caller's code/page. For example, you may want to show customized error page when error occurs, but all text printed by your code will break and stops this. One point is return boolean, as already said. But I personally prefer using exceptions for that (and even more -- I create special excepton which extends standard Exception class. This approach allows to differentiate one exception from another.).-
Get rid of chained method calls. Don't call
verifyDatabaseConnection() from verifyNullFields(), verifyDatabaseExist() from verifyDatabaseConnection() and so on. Your method should do only one thing as declared in their name.-
Rename
data member to something more meaningful.-
At your opinion: you may replace
$this->user == NULL) check to is_null($this->user)-
Also, when you will need to create similar class for PostgreSQL database (or abother DB) you should extract base class
Context
StackExchange Code Review Q#2325, answer score: 2
Revisions (0)
No revisions yet.