An online store, being converted from procedural to OOP

I just start to learn OOP, and it's far more interesting than procedural style.

I have a complete working online store written in procedural style. After realizing that my code is becoming very huge and very complex, I'm searching for methods to make my code more maintainable and more comfortable. One solution of course is to move to OOP style. But because I just started to understand how OOP work, I do not know which way to go.

con.php:

// Create a database connection
$connection = mysqli_connect(DB_SERVER, DB_USER, DB_PASS, DB_NAME);
// Test if connection occurred.
if(mysqli_connect_errno()) {
    die();
}

qfunctions.php

function get_user_info($user_id) {
// this function make a query for the user info
    global $connection, $query_error;;
    $query  = "SELECT *
                FROM users 
                WHERE users.user_id = {$user_id}";              
    $user_result = mysqli_query($connection, $query);

    // Test if there was a qurey error                                      
    if (!$user_result || mysqli_num_rows($user_result) == 0) {
        $query_error = $err_array["PROFILE_NOT_FOUND"]; 
    } else {
        return $user_result;    
    }   

}

I feel like this method is good but not the most efficient way possible, so I built a class for my database.

new_con.php

```
class MySQLDatabase {

private $connection;

function __construct() {
$this->open_connection();
}

public function open_connection() {
$this->connection = mysqli_connect(DB_SERVER, DB_USER, DB_PASS, DB_NAME);
if(!$this->connection) {
die("Database connection failed: " .mysqli_connect_errno());
} else {
$db_select = mysqli_select_db($this->connection, DB_NAME);
if(!$db_select) {
die("Database selection failed: " .mysqli_error($this->connection));
}
}
}

public function close_connection() {
if(isset($this->connection)) {
mysqli_close($this->connection);
unset($this->connection);
}
}

public f

At first I'm not mysqli guy and maybe somewhere I have mistake but the important part here Is the idea.

My advice to you is to throw exception instead of die or exit because you can catch them a lot easier. In your case you can put instantiation of MySQLDatabase in try/catch and if exception is occurred to show static page or whatever you wish.

class MySQLDatabase {

    private $connection;

    function __construct() {
        $this->open_connection();
    }

    public function open_connection() {
        $this->connection = mysqli_connect(DB_SERVER, DB_USER, DB_PASS, DB_NAME);
        if(!$this->connection) {
            throw new \Exception("Database connection failed: " .mysqli_connect_errno());
        }
    }
    /*
     * This function (mysqli_select_db) should only be used to change the default
     * database for the connection. You can select the default database with 4th
     * parameter in mysqli_connect().
     */
    public function select_db($db_name){
        mysqli_select_db($this->connection, $db_name);
        if(!$this->connection){
            throw new \Exception(" meaningfull message");
        }
        return $this;
    }

    public function close_connection() {
        mysqli_close($this->connection);
        unset($this->connection);
    }

    public function query($sql) {
        $result = mysqli_query($this->connection, $sql);
        return $this->confirm_query($result);
    }

    public function fetch_assoc($result_set) {
        return mysqli_fetch_assoc($result_set); 
    }

    private function confirm_query($result) {
        if(!$result) {
            throw new \Exception("Database query failed: " . mysqli_error($this->connection));
        }
        return $result;
    }

    public function free_result($result) {
        if($result) {
            return mysqli_free_result($result); 
        }
    }
}

I will make separate class for every table or group of futures(e.g. some kind of statistics) in separate class and all SQL queries are there. Important part of this is to not make validation in this class. You must consider that every parameter of method Is validated and correct.

class UserDomain
{
    protected $conn;

    public function __construct(MySQLDatabase $conn){
      $this->conn = $conn;
    }

    public function getUser($id){
        $sql = "SELECT * FROM users WHERE u.id = ?";
        $stmt = $this->conn->prepare($sql);
        $stmt->bind_param('i', $id);
        $stmt->execute();
        $tmp = array();
        while($res = $stmt->fetch()){
            $tmp[] = $res;
        }

        return $tmp;
    }

    public function setName($name, $user_id){}
    public function loginBy($user_id, $password){}
    public function changePassword($new_paswwrod, $user_id){}
    public function deleteUser($user_id){}
}

Here we have our class User. It can be tested very easy because your sql queries are now separated

// This class represents single user
class User {
    protected $id;

    protected $db;

    public function __construct($id, UserDomain $conn){
        $this->id = $id;
        $this->conn = $conn;
    }

    function getUserId(){
        return $this->id;
    }

    function changePassword($old_password, $new_password){

        if($this->conn->loginBy($this->id, $password)){
            // here you must validate new password against your business logic and if its true or pass try/catch you will update
            $this->changePassword($new_password, $this->id);
        }
    }

    function getUserInfo(){
        return $this->conn->getUserInfo($this->id);
    }
}

Finally I recommend you to watch this speak from Adam Culp he also have and book I think you will like it. And will give you nice ideas about refactoring legacy code and not write it

StackExchange Code Review Q#154556, answer score: 4