насколько все плохо ?

Только два вопроса:
1. По каким источникам учитесь
2. Где здесь ООП

 

crautcher
1. не ответ
2. нет, если в вашем коде убрать слово class по-сути программа нисколько не изменится. Вполне нормальный процедурный подход...

Достаточно сомнительно создавать таблицу бд в конструкторе класса
Откуда берётся переменная $sql?
Со стандартами оформления как-то не очень: http://www.opennet.ru/docs/RUS/php_code_standart/
UPD по стилю кодирования лучше сюда: http://framework.zend.com/manual/ru/cod ... style.html

Возьмите книгу что ли

 

crautcher
ООП подразумевает немного другое. Класс - это описание какого-либо объекта, у тебя же класс - это просто набор функций.

например, возьмём класс пользователя, каким его представляю я:

PHP:

1. <?php
2.  
3. class User {
4.  
5.   private $user = array(
6.       'user_id' => 0,
7.       'name' => '',
8.       'pass' => ''
9.     );
10.  
11.   // инициализация пользователя
12.   public function __construct($user_id = 0){
13.     if(self::exists($user_id)){
14.       $db = DatabaseClass::getInstance();
15.       $this->user['user_id'] = $user_id;
16.       $query = "SELECT `user_id`, `name`, `pass` FROM `users` WHERE `user_id`={user_id}";
17.       $this->user = $db->first($query, $this->user);
18.     }
19.   }
20.  
21.   // установка имени и пароля пользователя
22.   public function set_data($name, $pass){
23.     $this->user['name'] = $name;
24.     $this->user['pass'] = $pass;
25.   }
26.  
27.   // сохранение изменений пользователя
28.   public function save(){
29.     $db = DatabaseClass::getInstance();
30.     $query = "UPDATE `users` SET `name`={name}, `pass`=md5({pass}) WHERE `user_id`={user_id}";
31.     $db->query($query, $this->user);
32.   }
33.  
34.  
35.   // удаление текущего пользователя
36.   public function delete(){
37.     $db = DatabaseClass::getInstance();
38.     $query = "DELETE FROM `users` WHERE `user_id`={user_id}";
39.     $db->query($query, $this->user);
40.   }
41.  
42.  
43.   /* СТАТИЧНЫЕ ФУНКЦИИ */
44.  
45.   // проверка существования пользователя по айди
46.   private static function exists($user_id){
47.     $db = DatabaseClass::getInstance();
48.     $query = "SELECT COUNT(*) as `count` FROM `users` WHERE `user_id`={user_id}";
49.     $count = $db->first($query, array('user_id'=>$user_id));
50.     return $count['count'];
51.   }
52. }
53.  
54. ?>

в итоге я могу работать с пользователями как с объектами - создать объект нового пользователя, либо получить объект уже существующего пользователя по айди

PHP:

1. <?php
2.  
3. $new_user = new User();
4. $new_user->set_data('Gromo', '1234');
5. $new_user->save();
6.  
7. $old_user = new User(1);
8. $old_user->delete();

 

Gromo
Симпатичный код, но всё же он не является примером ООП

 

Gromo
Улыбнуло))

PHP:

1. <?php
2. if(self::exists($user_id)){
3.        $db = DatabaseClass::getInstance();
4. //...................
5. private static function exists($user_id){
6.      $db = DatabaseClass::getInstance();
7.  

И зачем все эти телодвижения с инстансом DatabaseClass, есло можно один раз в конструкторе вызвать

 

alexfer
Дело вкуса, нет? (В static-методе вызвать конструктором? о боже..)

 

Неа. При чем здесь вкус? Да и статик там нафиг не нужен.

 

Он прав, $this->db = .. в конструкторе лучше, а еще лучше $this->setDb() в конструкторе и $this->getDb() везде.

 

MiksIr
Лучше, чем? Может сразу реестр?

 

Сервис-локаторы не многим лучше Синглтона, но я о другом. Речь об использовании переданного внутри класса.

Лучше тем, что если захочется перейти на этот самый реестр, или вообще переписать класс для использования вместе с инжекцией зависимостей - придется поправить одно место, где делается setDb вместо путешествия по классу в поисках, где же еще этот db дергается.

 

А зачем геттер? по моему private $db; справиться с этим нормально

PHP:

1.  
2. <?php
3. $this->db = DatabaseClass::getInstance();
4. $this->db->query($sql);
5.  

 

Это уже долгий холивар. Как минимум сеттер позволит сделать type hinting
protected function setDb( Db $db ) {

 

MiksIr
Согласен. Этот вариант рациональнее

 

это мой взгляд на вещи, предпочитаю не хранить ссылку на объект БД в объектах других классов.
тем более, что в оригинале я получаю реестр, где хранится не только объект БД, но и другие данные.

а здесь лишь привёл часть класса для наглядного примера

предпочитаю в конструктор передавать только те данные, которые относятся к объекту, не создавая лишних свойств

я предпочёл сделать единый интерфейс для различных классов баз данных, и через функцию получать
объект именно того типа базы, который сейчас используется в системе

 

это не правильно

они не избыточны

жаль интерфейса не видно....

 

неважно. мне не нравится, потому так не делаю

а видеть и не нужно. всё и так гут.

все классы бд должны поддерживать стандартный sql (на котором собственно и пишутся запросы в бд),
делать разбор запроса и экранирование переменных, поддерживать набор методов интерфейса

 

alexfer, здесь не тот форум, что бы спорить о том, что правильно, а что говнокод, хоть и ООП.