OO and non-OO usage of Mysqli

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;
}

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?

• You don't really need to assign so many methods to assigning things. You won't really ever retrieve those again, so dedicating methods to assigning them is not necessary, I don't think. – Rasclatt Dec 30 '17 at 6:01
• @Rasclatt So one method to rule'em all? – Veljko Stefanovic Dec 30 '17 at 6:16
• Post edited according to advice given by @Rasclatt . – Veljko Stefanovic Dec 30 '17 at 6:22
• I am not sure if I am going to write a distinct answer, so just a few pointers. First of all, you shouldn't write some code, or use some paradigm only for sake of it, just because everyone else is doing it. Your new code should be better than old one. Do you see any benefit in your class? If so, what is it? Regarding the class itself, I've got an article, Your first database wrapper's childhood diseases, it is definitely worth to read – Your Common Sense Dec 30 '17 at 10:00
• Also I wrote an answer to the similar question recently, you may find it useful: codereview.stackexchange.com/questions/183801/… – Your Common Sense Dec 30 '17 at 10:07

Most of the code in this class just makes no sense and written only because it has been shown in some tutorial as an example for some other class.

In case of a database connection class, there is no use for the database credentials' getters. You never need a database host anywhere outside of the database class. Hence, getServername() method will never be used. Neither such class variables are actually needed as you only need them in the constructor. And of course, there shouldn't be a setVal() method whic is way too localised to be a part of the database class.

From the above, it can be clearly seen that there is no useful functionality in this class, and for a reason: mysqli is already a class, and not the worst one.

In your case, it would be much, much better to learn how to use vanilla mysqli first.

OO usage of mysqli is simple. All you need to know is the main rule of the database interaction in PHP:

A database connection must be created strictly once!

Therefore, you have to create the mysqli object first, and then use this instance for all the database interactions in your script. The simplest code for this would take just three lines:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); // enable error reportig
$conn= new mysqli($host, $user,$pass, $db,$port); // the actual connection
$conn->set_charset($charset); // set connection charset


After this, you can use the instance of mysqli class to run SQL queries OO style:

$sql = "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 )";$conn->query($sql);  And it would be the perfect example of using mysqli OO style. Other OO usage examples of mysqli can be found in this answer • I don't have edit privs but there is a typo "whic" – Dharman Feb 25 at 21:27 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 MyApp\Database; # 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(\MyApp\Logging\Model$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 MyApp\Query; # 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 \MyApp\Database\Model as Connection; class Controller { private$con,
$query,$Log,
$Db; public function __construct(\MyApp\Logging\Model$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_NAME','dbname');
# Create the logging class
$Logger = new \MyApp\Logging\Model(); # 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 \MyApp\Query\Controller($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.

• A little review, if you let me. You are making a very common mistake, for some reason trying to handle a few random errors and leaving all other errors unhandled. That makes your code inconsistent and WET. What if, for example, a call to $this->con->query($sql); will result in the error? There is no code to handle it. And it shouldn't be. As well as there shouldn't be a code to handle an erroneous query. You should throw an useful exception instead, whereas logging and whatever 'An error occurred.' message processing should occur elsewhere, in the centralized manner, making your code shorter – Your Common Sense Dec 30 '17 at 10:06
• Besides, your code creates a new database connection for the every object, which is a BIG no-no, it will result in a database server error "Too many connections". You are sending a Logger instance in the constructor but do not follow this pattern for the database class. is there any reason for that? Also, given the usage pattern of this class, I don't see why there is a getConnection method. Why cannot createConnection return it already? – Your Common Sense Dec 30 '17 at 10:10
• @YourCommonSense Thanks for the review, if you allow me the opportunity for rebuttal. In the example I have stated to pass the query engine which initiates the connection inside it's constructor into proceeding classes and not to create it again so multiple connections should not be an issue unless that is not adhered to. Since the user is just exploring OOP, my example is really only demonstrating a one layer script to give the OP the idea of separating DB create from a query engine, I believe they are two different things. Extend would be a better plan to remove redundancy, I agree there. – Rasclatt Dec 30 '17 at 17:06
• @YourCommonSense Also, I agree with you logging would be better served centralized to capture and separate logging from visual errors, however I am not demonstrating how to create a framework and am not trying to create a class matrix to cover every scenario, rather some concepts on dependency injection and throwing errors/logging rather than dying/echoing, etc. inside a Model. I also believe useful exceptions should not include MySQL warnings when a query failed straight from the DB. I believe the average user just needs to know something is wrong with the site, not a mysql error code/string. – Rasclatt Dec 30 '17 at 17:11
• Ok, it seems you misunderstood my comment on the error logging. You seem to be under the impression that exceptions are inevitably shown to the site user. In fact it is not so. With a centralized error handling your 'An error occurred' will be shown to a site user all right, even if a useful mysqli exception is thrown. You are making a common mistake by logging an error message inside if a model. It makes your code inflexible and hard to debug. – Your Common Sense Dec 31 '17 at 7:42