I have a class for each table in the database.
I use some static methods inside them for interaction. and A global value inside each mothod (that is the database connection).
I know that it’s not the right way. It’s not a good design. How do you guys do that? Please tell me what are my mistakes? Thanks.
an example:
<?php
class QuestionsTable
{
private static $db_columns = ['question_id', 'user_id', 'subject', 'created_at']; // The columns in the table.
public static function getValue($question_id, array $which_columns)
{
global $dbc;
$query = "SELECT ";
foreach ($which_columns as $value) {
if (in_array($value, self::$db_columns)) {
$query .= "{$value}, ";
}
}
$query .= "FROM questions "
. "WHERE question_id={$question_id}";
$query = str_replace(', FROM', ' FROM', $query);
$result = mysqli_query($dbc, $query);
$data = mysqli_fetch_array($result, MYSQLI_ASSOC);
return $data;
}
public static function create($user_id, $subject)
{
global $dbc;
$query = "INSERT INTO questions (subject, user_id, created_at) "
. "VALUES(?, ?, UTC_TIMESTAMP())";
$stmt = mysqli_prepare($dbc, $query);
mysqli_stmt_bind_param($stmt, 'si', $subject, $user_id);
mysqli_stmt_execute($stmt);
return true;
}
public static function delete($question_id)
{
global $dbc;
$query = "DELETE FROM questions WHERE question_id=? LIMIT 1";
$stmt = mysqli_prepare($dbc, $query);
mysqli_stmt_bind_param($stmt, 'i', $question_id);
mysqli_stmt_execute($stmt);
return true;
}
}
I hope this reply isn’t too late - I only just spotted your question while looking back for unanswered threads.
You’re right about the global variable - it’s generally a good idea to avoid using them. For a situation like this, I’d suggest switching to the object oriented method of working with the mysqli extension, then you can pass your DB connection object into your class:
class QuestionsTable
{
private $db;
public function __construct(mysqli $db)
{
$this->db = $db;
}
// ...
}
$mysqli = new mysqli("example.com", "user", "password", "database");
$questions = new QuestionsTable($mysqli);
As you can see from the example above, the class now needs to be instantiated, so the static methods need converting to instance methods.
You could do the whole thing as a static class still, but it’s easier to write more decoupled and testable code if you don’t.
I’d also suggest using a prepared statement for your getValue() method too, as this is more secure. It’s also possible to tidy up the method a little by using a couple of built-in functions:
Seperate concerns. Create a class for the connection and database interaction and a seperate class for each table. You can include a manager class to create the batch classes on the fly.
Well, I suppose there’s nothing to stop you using the procedural method, but if you’re already working with objects then it’s more consistent not to mix the two.
If you use oop you can use the dry principle. Instead of debugging the code on a per page basis you just debug that one class and it fixes the issue. It’s also going to reduce the lines of code you write on the pages that use the class. It also makes working with the database easier. You can even chain methods if implemented right.