Mainly used Stack Overflow, so first time over here for a code review. I am trying to make some enhancements to a legacy application. I have some minor experience using Laravel, but this application is using quite a heavy procedural approach, and I am struggling to work out the best way to structure thing in an OO sense. I have tried to adopt a repository pattern just to separate tasks between the separate entities, and I would be keen to get some feedback.
Firstly I have my basic User class model with its constructor and getter methods.
<?php
class User {
// Properties
private $id;
private $username;
private $password;
private $email_address;
private $first_name;
private $last_name;
private $group_id;
function __construct($username, $password, $email_address, $first_name, $last_name, $group_id, $id = 0) {
$this->id = $id;
$this->username = $username;
$this->password = $password;
$this->email_address = $email_address;
$this->first_name = $first_name;
$this->last_name = $last_name;
$this->group_id = $group_id;
}
// Methods
public function getID(){
return $this->id;
}
public function getUsername(){
return $this->username;
}
function getPassword(){
return $this->password;
}
function getEmailAddress(){
return $this->email_address;
}
function getFirstName(){
return $this->first_name;
}
function getLastName(){
return $this->last_name;
}
function getGroupID(){
return $this->group_id;
}
}
Then I have my MySQL connector, I am a bit unsure about this due to the existence of MySQLi, it just feels like I am duplicating everything inside it. Though at least its allowing me to only specify the connection in one place.
<?php
include('config.php');
class MySQLConnector {
private $dbUser = DBUSER;
private $dbPass = DBPWD;
private $dbName = DBNAME;
private $dbHost = DBHOST;
private $link;
function __construct() {
$this->link = new mysqli($this->dbHost, $this->dbUser, $this->dbPass, $this->dbName);
}
function __destruct() {
$this->close();
}
function close() {
$this->link->close();
}
public function query($query) {
return $this->link->query($query);
}
// Get the data return int result
function escapeString($query) {
return $this->link->escape_string($query);
}
function prepare($query) {
return $this->link->prepare($query);
}
function numRows($result) {
return $result->num_rows;
}
// Gets array of query results
function fetchAssoc($result) {
return $result->fetch_assoc();
}
// Fetches all result rows as an associative array, a numeric array, or both
function fetchArray($result, $resultType = MYSQLI_ASSOC) {
return $result->fetch_array($resultType);
}
// Get a result row as an enumerated array
function fetchAll($result, $resultType = MYSQLI_BOTH) {
return $result->fetch_all($resultType);
}
// Free all MySQL result memory
function fetchRow($result) {
return $result->fetch_row();
}
}
I think defined an interface called Repository that certain ones will follow.
<?php
interface Repository {
public function getById($id);
public function getAll();
public function insert(Object $object);
public function update(Object $object);
public function delete($id);
}
Then lastly I have the UserRepository class, which is acting like a middleman between the User and MySQLConnector classes. I am not sure if I have taken the correct approach here, as it feels like I am duplicating code in places such as the insert and update statements, but I can't think of a way to optimise it further.
<?php
require_once('Repository.php');
require_once('MySQLConnector.php');
class UserRepository implements Repository {
private $table = 'user_list';
private $db_obj;
public function __construct() {
$this->db_obj = new MySQLConnector();
}
public function getById($id) {
$query = 'select * from ' . $this->table . ' where id = '.$id;
$result = $this->db_obj->query($query);
return $this->db_obj->fetchAssoc($result);
}
public function getAll() {
$query = 'select * from ' . $this->table;
$result = $this->db_obj->query($query);
return $this->db_obj->fetchAll($result);
}
public function insert($object) {
$username = $this->db_obj->escapeString($object->getUsername());
$firstName = $this->db_obj->escapeString($object->getFirstName());
$lastName = $this->db_obj->escapeString($object->getLastName());
$email = $this->db_obj->escapeString($object->getEmailAddress());
$groupID = $this->db_obj->escapeString($object->getGroupID());
$password = password_hash($this->db_obj->escapeString($object->getPassword()), PASSWORD_DEFAULT);
$query = "insert into " . $this->table . " (username, first_name, last_name, email_address, password, group_id) values (? , ? , ? , ? , ?, ? )";
$stmt = $this->db_obj->prepare($query);
$stmt->bind_param('sssssi', $username, $firstName, $lastName, $email, $password, $groupID);
return $stmt->execute();
}
public function update($object) {
$id = $this->db_obj->escapeString($object->getID());
$username = $this->db_obj->escapeString($object->getUsername());
$firstName = $this->db_obj->escapeString($object->getFirstName());
$lastName = $this->db_obj->escapeString($object->getLastName());
$email = $this->db_obj->escapeString($object->getEmailAddress());
$groupID = $this->db_obj->escapeString($object->getGroupID());
$password = password_hash($this->db_obj->escapeString($object->getPassword()), PASSWORD_DEFAULT);
$query = "UPDATE " . $this->table . " SET username = ?, first_name = ?, last_name = ?, email_address = ?, password = ?, group_id = ? WHERE id = ?";
$stmt = $this->db_obj->prepare($query);
$stmt->bind_param('sssssii', $username, $firstName, $lastName, $email, $password, $groupID, $id);
return $stmt->execute();
}
public function delete($id) {
$id = $this->db_obj->escapeString($id);
$query = "DELETE FROM " . $this->table . " WHERE id = ?";
$stmt = $this->db_obj->prepare($query);
$stmt->bind_param('i', $id);
return$stmt->execute();
}
}
I am aware that I am missing some validation and error checking throughout the code. Regarding validation I want to do some checks of the format of data being supplied, whilst I need to do some further research regarding the appropriate ways for trying and catching errors. Would just be grateful to know if I am going down the correct approach, and any suggestions people may have.