abstract class MainModel { const TABLE = ''; public static function getAll(){ $dbh = DB::getInstance(); $sql = 'SELECT * FROM '.static::TABLE; return $dbh->query($sql); } public static function getOne($id){ $dbh = DB::getInstance(); $sql = 'SELECT * FROM '.static::TABLE.' WHERE id='.$id; return $dbh->query($sql); } public static function deleteOne($id){ $dbh = DB::getInstance(); $sql = 'DELETE FROM '.static::TABLE.' WHERE id='.$id; return $dbh->execute($sql); } }
- inherit? - tCode
- The model itself does not need to be made static. Let it be a regular object that is created by a factory. Which will transfer the connection from the database to the constructor - Ipatyev
- oneYou have violated the SOLID principle, and this is the principle of dependency inversion. Your MainModel methods implicitly depend on the availability of the DB and its internal state. You cannot use the MainModel without a DB, and the DB supposedly requires connecting to the database. - Firepro
|
1 answer
static private $connect = null; static public function getConnect() { if(!is_null(self::$connect)) { return self::$connect; } self::$connect = DB::getInstance(); return self::$connect; }
And in general, why do you need a static class is not clear ...
all the same, I transformed it so
abstract class MainModel { private $table = null; private $connect = null; private function __construct() { $this->_init(); } abstract protected function _init(); public function getAll(){ $sql = 'SELECT * FROM '.$this->getTable(); return $this->getConnect()->query($sql); } public function getOne($id){ $sql = 'SELECT * FROM '.$this->getTable().' WHERE id='.$id; return $this->getConnect()->query($sql); } public function deleteOne($id){ $sql = 'DELETE FROM '.$this->getTable().' WHERE id='.$id; return $this->getConnect()->execute($sql); } public function setTable($table) { $this->table = $table; return $this; } public function getTable() { return $this->table; } final public function getConnect() { if(!is_null($this->connect)) { return $this->connect; } $this->connect = DB::getInstance(); return $this->connect; } }
- For my taste, here are the same balls, only in profile :) I would make the usual protected variable, and connect in the constructor. - Ipatiev
- @ Ipatyev init made it abstract so that we need to override it in the child classes in order to install the same label, since the constructor can also forget to override it, and with the abstract class and ide will highlight and php error will throw. About getConnection () in case you want to get a connection from another class and execute the query (it doesn't sound nice but it happens) and you can assign a value to a variable somewhere in the child class, which will also lead to an error. - Naumov
- @ Ipatiev, in fact, it can even be final, so as not to override - Naumov
|