2
\$\begingroup\$

i just wanted to confirm if my codes below is correct. Feel free to make corrections.

<?php
class createConnection
{
var $host="localhost";
var $username="root";
var $password="";
var $database="wilbert";
var $myconn;



function connectToDatabase()
{
$conn = mysql_connect($his->host,$this->username,$this->password);
if (!$conn)
{
    die ("Cannot connect to the database");
}

else
{
    $this->myconn = $conn;
    echo "Connection established!";
}
return $this->myconn;

}
function selectDatabase()
{
mysql_select_db($this->database);
if(mysql_error())
{

    echo "Cannot find the database ".$this->database;
}
else
{
echo "Database selected..";
}

}
function selectData()
{
mysql_select_db($this->database);

$result = mysql_query(" SELECT a.student_id, a.FirstName, b.First_Name FROM Student a        LEFT JOIN teacher b ON a.student_id = b.student_id");

echo "<table border='1'>
<tr>
<th>Student_ID</th>
<th>First_Name</th>
<th>First_Name</th>
</tr>";

while($row = mysql_fetch_array($result))
  {
  echo "<tr>";
  echo "<td>" . $row['student_id']. "</td>";
  echo "<td>" . $row['FirstName'] . "</td>";
  echo "<td>" . $row['First_Name'] . "</td>";
  //echo "<td>" . $row['student_id'] . "</td>";
  echo "</tr>";
  }
echo "</table>";
}

function closeConnection()
{
mysql_close($this->myconn);
echo "connection closed!";
}

}
?>

and here is my object.

<?php

include ('phpclass.php');

$connection = new createConnection(); //i created a new object

$connection->connectToDatabase(); // connected to the database

echo "<br />"; // putting a html break

$connection->selectDatabase();

echo "<br />";

$connection->selectData();
//$connection->closeConnection();// closed connection
?>
\$\endgroup\$
5
  • 5
    \$\begingroup\$ I'd probably avoid using a deprecated library (i.e. mysql_*) too. \$\endgroup\$
    – BenM
    Feb 27, 2013 at 18:20
  • \$\begingroup\$ Either you should have cut and pasted a lot more carefully or shown us the error which occurred when you ran this ( mysql_connect($his->host,$t... ) \$\endgroup\$
    – symcbean
    Mar 5, 2013 at 21:09
  • \$\begingroup\$ @symcbean - the above codes, my original codes are working but when I try the answers below, it doesnt work anymore :( \$\endgroup\$
    – lexter
    Mar 6, 2013 at 0:04
  • 1
    \$\begingroup\$ @lexter: really? What is $his ? \$\endgroup\$
    – symcbean
    Mar 6, 2013 at 9:30
  • \$\begingroup\$ @symcbean - it's a typo.. it must be $this \$\endgroup\$
    – lexter
    Mar 7, 2013 at 3:45

1 Answer 1

7
\$\begingroup\$

I am a fan of loose coupling so I would suggest that you take the values of host, username, password and database out of your class. Pass the values through the constructor and set them in the constructor. This makes it easier to reuse your code.

Second, in classes such as one that creates a connection to a database I would not "echo" stuff. You better throw exceptions on error or return meaningful values to the caller.

Last, I'd suggest other names. This might be nitpicking but this has more meaning to me:

$connection = new DatabaseConnection();
$connection->open();
$connection->connectDatabase();

So, wrapped up I would have something like:

<?php
class DatabaseConnection
{
    var $host;
    var $username;
    var $password;
    var $database;
    var $myconn;

    function __construct($dbhost, $user, $pass, $db)
    {
        $this->host = $dbhost;
        $this->username = $user;
        $this->password = $pass;
        $this->database = $db;
    }

    function open()
    {
        $conn = mysql_connect($his->host,$this->username,$this->password);
        if (!$conn)
            throw new Exception('Error establishing a connection.');
        else
            $this->myconn = $conn;

        return $this->myconn;
    }

    function connectDatabase()
    {
        mysql_select_db($this->database);
        if(mysql_error())
            throw new Exception("Cannot find the database " . $this->database);
    }

    function selectData($query)
    {
        mysql_select_db($this->database);
        $result = mysql_query($query);
        return $result;
    }

    function closeConnection()
    {
        mysql_close($this->myconn);
    }
}
?>

Usage:

$connection = new DatabaseConnection("localhost", "root", "", "wilbert");
$connection->open();
$connection->connectDatabase();
$connection->selectData("SELECT a.student_id, a.FirstName, b.First_Name FROM Student a LEFT JOIN teacher b ON a.student_id = b.student_id");

See how the other names make more sense? Try to avoid verbs as a classname. Methods can and most likely will be or contain verbs, but not classnames. Also, leaving the query out of the class leaves you the option to use the same code for other queries.

Note that this is no perfect code, I was only suggesting what might be changed. Hope this helps! :)

Edit:

You call the function selectData() and pass the query-string as a parameter. This changes nothing, it gives the same result as your code. Only in my case you can pass other queries to the function whereas your function is limited to the embedded query.

Now for the result. This has also to do with the decoupling of your connection-class and what will be shown to the user. I showed u the usage of the class, the code below is how you can process the result. Create a function (but not in the connection class!) that returns the result in a table:

function resultTable($sqlResult)
{
    $table = "<table border='1'>";
    $table .= "<tr><th>Student_ID</th><th>First_Name</th><th>First_Name</th></tr>";

    while($row = mysql_fetch_array($sqlResult))
    {
        $table .= "<tr><td>" . $row['student_id']. "</td>";
        $table .= "<td>" . $row['FirstName'] . "</td>";
        $table .= "<td>" . $row['First_Name'] . "</td></tr>";
    }
    $table .= "</table>";

    return $table;
}

And the usage:

$connection = new DatabaseConnection("localhost", "root", "", "wilbert");
$connection->open();
$connection->connectDatabase();
$result = $connection->selectData("SELECT a.student_id, a.FirstName, b.First_Name FROM Student a LEFT JOIN teacher b ON a.student_id = b.student_id");
echo resultTable($result);

You could create a new class in which you define all sorts of methods like these to process a result and return it to the view-file where you will show it on the screen. Hope this clarifies it. :)

\$\endgroup\$
6
  • \$\begingroup\$ in the part of function selectData($query) { mysql_select_db($this->database); $result = mysql_query($query); return $result; } where will the $query get its values? is it possible to show the result in a table, because in my codes above. the results of the LEFT JOIN were embedded in an html table :) \$\endgroup\$
    – lexter
    Feb 28, 2013 at 1:22
  • \$\begingroup\$ Check my edited answer! ;) \$\endgroup\$
    – Abbas
    Feb 28, 2013 at 8:10
  • \$\begingroup\$ how will the class and object be connected in your codes?where is the "include();" line? \$\endgroup\$
    – lexter
    Mar 1, 2013 at 8:03
  • \$\begingroup\$ include('DatabaseConnection.php'); and then the lines of code from the section usage. For simplicity you can place the resultTable-function in the view-file. Otherwise place it in a separate file and include that file as wel with an include(). Then simply call the function with the SQL-result. \$\endgroup\$
    – Abbas
    Mar 2, 2013 at 0:26
  • \$\begingroup\$ echo resultTable($result); $result doesnt have any values. am i correct? \$\endgroup\$
    – lexter
    Mar 4, 2013 at 3:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.