Gfrktyl

I'm starting to learn the object oriented aspect of PHP. In this regard I made a small "exercise" of sorts "translating" one of my PHP functions into OO mode of PHP. I'm asking if there are there any recommendations you can give me concerning my predicament.

Example of non-OO PHP:

<?php

function pristup($servername, $username, $password, $dbname, $sql){



    $conn=new mysqli($servername, $username, $password, $dbname);



    if($conn->connect_error){



        die("Neuspela konekcija: ".$conn->connect_error);



    }



    $result = $conn->query($sql);

    if ($result == TRUE) {

    //echo "Uspela konekcija";

} else {

    echo "Neuspešno izvršavanje upita: " . $conn->error;

}

    return $result;



    $conn->close();



}



?>

Example of this done in an object oriented manner:

<?php



class konekcija{



    private $servername;

    private $username;

    private $password;

    private $dbname;

    private $sql;



    //Setter functions

    public function setVal($par1, $par2, $par3, $par4, $par5){ //Setovanje vrednosti za upit.

    $this->servername = $par1;

    $this->username = $par2;

    $this->password = $par3;

    $this->dbname = $par4;

    $this->sql = $par5; 



}



    //Getter functions

    public function getServername() { 

        return $this->servername;

    }



    public function getUsername() { 

        return $this->username;

    }



    public function getPassword() { 

        return $this->password;

    }



    public function getDBname() { 

        return $this->dbname;

    }



    public function getSQL() { 

        return $this->sql;

    }



    //Function that executes query.

    public function pristup($server_name, $user_name, $pass_word, $db_name, $sql_query){



        $conn=new mysqli($server_name, $user_name, $pass_word, $db_name);



        if($conn->connect_error){



            die("Neuspela konekcija: ".$conn->connect_error);



        }



        $result = $conn->query($sql_query);



        if ($result == TRUE){



        echo "Uspela konekcija";



        } 

        else{



                echo "Neuspešno izvršavanje upita: " . $conn->error;



            }



        return $this->$result;



        $conn->close();



    }



}





$kon = new konekcija(); //Creation of an object.



//Setting values.

$kon -> setVal("localhost", "root", "", "test", "CREATE TABLE example(

            id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY,

            firstname VARCHAR(30) NOT NULL,

            lastname VARCHAR(30) NOT NULL,

            email VARCHAR(50),

            reg_date TIMESTAMP

            )");



//Getting values and inserting them into class method.

$kon -> pristup($kon->getServername(), $kon->getUsername(), $kon->getPassword(), $kon->getDBname(), $kon->getSQL());



?>

This works, and I'm asking if this code is any good. Is there a better/more established way of doing this?

I think you might be better served breaking what you have into a database class and a query-type class that will use the database class to accomplish queries and return rows, maybe something like:

/vendor/MyApp/Database/Model.php

<?php

namespace MyAppDatabase;

# You may want to implement an interface here or abstract class to allow for

# different connection types (looking to the future)

class Model

{

    protected $conn,

              $Log = false;

    # I think you might want to pass a logging class to save server/connection errors

    public function __construct(MyAppLoggingModel $Log)

    {

        $this->Log = $Log;

    }



    public function createConnection($host,$username,$password,$dbname)

    {

        $this->conn = new mysqli($host, $username, $password, $dbname);

        if(!empty($this->conn->connect_error)) {

            # Here is where you want to log potentially sensitive errors.

            $this->Log->saveError($this->conn->connect_error);

            # Just let the user know something has gone wrong, they don't need

            # to know the error code and cryptic messages doled out by MySQL

            throw new Exception('Database can not connect.');

        }



        return $this;

    }



    public function getConnection()

    {

        if($this->conn instanceof MySQLi)

            return $this->conn;



        throw new Exception('Database connection not yet set.');

    }



    public function closeConnection()

    {

        if($this->conn instanceof MySQLi)

            $this->conn->close();

    }

}

/vendor/MyApp/Query/Controller.php

<?php

namespace MyAppQuery;

# You can use the database model to create a contained connection

# You could extend this DB class to make this Query class, I am just going

# to use it instead though

use MyAppDatabaseModel as Connection;



class Controller

{

    private $con,

            $query,

            $Log,

            $Db;



    public function __construct(MyAppLoggingModel $Log)

    {

        # You probably want to log failed queries, so use the same logging class

        $this->Log = $Log;

        # Pass that class to the connection

        $this->Db = new Connection($this->Log);

        # Create the connection here using the CONSTANTS in your config file

        # Assign the connection internally to this class

        $this->con = $this->Db->createConnection(DB_HOST,DB_USERNAME,DB_PASSWORD,DB_NAME)

            ->getConnection();

    }

    # You probably want to make a way to bind parameters

    # Also note, I use PDO, so is copied from yours...

    public function query($sql)

    {

        $this->query = $this->con->query($sql);

        if($this->query != TRUE) {

            # Same as connection, you need to know what really happened,

            # but your user doesn't

            $this->Log->saveError($conn->error);

            throw new Exception('An error occurred.');

        }



        return $this;

    }

    # Here is where you would send back the rows (probably requires a while() here)

    public function getResults()

    {

        return $this->query;

    }

    # You may want to be able to get the raw connection, who knows...

    public function getConnection()

    {

        return $this->con;

    }

}

You won't ever (or very rarely) have to retrieve the database credentials so you don't really need to dedicate a bunch of methods to assign those.

Depending on your app, you can do a contained engine without having to always add your db credentials to the main database class:

<?php

# I might do a config file that has this kind of info or an editable php 

# array with the credentials in it. Something that can not be directly 

# accessed by a user

define('DB_HOST','localhost');

define('DB_USERNAME','root');

define('DB_PASSWORD','');

define('DB_NAME','dbname');

# Create the logging class

$Logger = new MyAppLoggingModel();

# Create the query engine, add in a logging class to keep track of errors

# You don't want to show the user the actual errors, those are best kept

# for the administrator's eyes. The user just needs to know something is not

# working

# One note here, you would want to pass $qEngine to all classes that use

# the query engine / database connection, don't make more instances of this

$qEngine = new MyAppQueryController($Logger);

# Create a query and get the results

# This is to allow for writing queries and fetch rows (the getResults() method)

$results = $qEngine->query("SELECT * FROM `users`")->getResults();

Finally, you don't really want to echo anything except in your view so you can use a try to catch errors and print them in the view, not inside the data fetching methods. Depending on the error, die() might be a bit much. If you throw an Exception, depending on the severity of it, you can print a designed page that has your error printed to the page instead of the rather bleak look of a die('String of text here.').

Anyway, I am no hardcore expert, there are lots of books and articles regarding patterns and such that would be helpful to you. These are some things I have done in the past (and still do for the most part). Also note, I haven't tested this specifically, it's more to just give you some ideas.