Правильно ли я использую ООП?

Question

[zeuss56]

• Все классы разложены по папкам согласно неймспейсам (если используются) в папке class. Подгружаются autoloader'ом:
  

  # константы
  const DS = DIRECTORY_SEPARATOR;
  
  # установка текущей директории
  chdir(__DIR__);
  
  # инициализация автозагрузки
  spl_autoload_register(function($classname) {
    $filepath = 'class'.DS.str_replace('\\', DS, $classname).'.php';
    return file_exists($filepath) ? include_once($filepath) : false;
  });

  
  
• Сначала создаём интерфейс, затем имплементируем его (не всегда нужно).
  
• Если есть переменная, с которой нужно долго работать, как ссылка на БД или на файл, делаем для неё приватную переменную и сеттер (часто роль сеттера выполняет конструктор):
  

  private $db;
  
  function __construct(DB $db) {
    # установка класса БД
    $this->db = $db;
  }

  
  
• Естественно, повторяющийся код выносим в отдельные методы. Если есть похожие методы в разных классах, выносим их в отдельный класс и используем или наследуем этот класс, смотря насколько общий функционал.
  
  
• Константы определяются вне классов. Константы классов лишь ссылаются на уже определенные снаружи:
  

  const LOCALES_TABLE = 'com_locales';
  
  class Locale {
    private $localestable = LOCALES_TABLE;
    # ...
  }

  
  
• Использую serialize()/unserialize() для хранения массивов в константах. Кажется, в PHP7 это уже не нужно, но не помню. Хоть это к ООП не относится, но интересно:
  

  const ABS_TABLES = serialize([
    'com_abs_str',
    'com_abs_int'
  ]);
  
  class ABS_Tables {
    private $tables;
  
    function __construct() {
      $tables = &$this->tables;
      $tables = unserialize(ABS_TABLES);
    }
    # ...
  }

  
  
• В каждом методе используемые переменные линкуются чтобы не писать $this-> при обращении. Насколько это правильно?
  

  function GetLanguageByID($langid) {
    # линки
    $db         = &$this->db;
    $langstable = &$this->langstable;
    # ...
  }

  
  UPD (некоторые товарищи не "догоняют"):
  Во многих языках если не нашлось переменной с опеределённым именем определённой в текущем пространстве переменных, она ищется выше. В PHP такого нет. Если я обращаюсь к `$anyvar` из метода класса, то он не найдёт переменную и выкинет нотис. А я хочу обращаться к `$anyvar` класса. Вот и переименовываю `$this->anyvar` в `$anyvar` чтобы меньше писать `$this->` внутри метода. А ссылка - чтобы и чтение и запись в переменную работала.
  
• Для проверки корректности ввода я использую такую функцию чтобы не проверять вручную:
  

  private function CheckArgs(array $inputs) {
    foreach ($inputs as $input)
      if ($input === [] || $input === '' || $input === null)
        return false;
      return true;
  }
  
  function InsertValues($table, array $keys, array $values_arr) {
    # проверка всех аргументов
    if (!$this->CheckArgs(func_get_args()))
      return;
    # ...
  }
  
  function SelectValues($table, array $wherekeys = [], array $wherevalues = []) {
    # проверка определенных аргументов
    if (!$this->CheckArgs([$table]))
      return;
    # ...
  }

  
  

Comments

[Sergej]

Начните пользоваться composer

[zeuss56]

Doc: спасибо, он хорош, но при чём тут он?

Answer 1

[Антон Иванов]

Ну как вам сказать ... На мой взгляд вы бы лучше взяли современный PHP-фрейворк и стали с ними работать, так вы быстрее разберетесь в ООП. Т.к. фрейворк написан достаточно умелыми людьми (например, Yii2, Laravel и т.п.).

Вы не озвучили цель вашего кода? Чего вы хотите добиться? Просто разобраться в ООП?

Теперь по пунктам, что сразу бросилось в глаза. Вы наверно не слышали про паттерны проектирования. Почитайте про них - это тот самый бест-практисес в ООП.

Если есть переменная, с которой нужно долго работать, как ссылка на БД

Вот тут очень хочется применить паттерн Singleton.

В каждом методе используемые переменные линкуются - просто устал постоянно писать $this->

Если я не ошибаюсь определение каждой переменной - это выделение свободной памяти. Так что ваш подход сомнителен. От "усталости" поможет хороший принцип KISS (Keep It Simple Stupid) - сделайте каждый метод или функцию очень маленькой, компактной, выполняющей только одну единственную задачу.

Использую serialize()/unserialize() для хранения массивов в константах

Ой, а поделитесь, для чего вы это делаете?

Как то так. Можете пинать.

Comments

[zeuss56]

Вы не озвучили цель вашего кода? Чего вы хотите добиться? Просто разобраться в ООП?

Цель вопроса - узнать, правильно ли я пишу код на ООП. А код - это выдержки из продакшена. В компании ничего против моего стиля не имеют, что странно.

Вы наверно не слышали про паттерны проектирования

Слышал, разбирал и синглтон, и MVC, и MVVM, не пригодился пока ни один - мало писал.

Использую serialize()/unserialize() для хранения массивов в константах

Ой, а поделитесь, для чего вы это делаете?

Для хранения массивов в константах, не через глобал же обращаться к переменным - дурной тон же. Предлагаете через регистр объявлять список? Или хранить всё внутри класса?

От "усталости" поможет хороший принцип KISS

KISS, DRY - это настолько само собой разумеется, что не стоит и упоминать. А к неудобному синтаксису в PHP это никак не относится. Я полагаю, вы не совсем поняли, зачем здесь линковать переменные.
Спасибо, что указали на ошибки, но как их исправлять - я не понял.

[Stalker_RED]

Вадим Егоров: Я вот тоже не понимаю зачем вы их линкуете. Точно также непонятно, зачем вы делаете штуки типа const DS = DIRECTORY_SEPARATOR;, ведь код пишется один раз а читается - много раз. И посторонний человек не догадается сразу что значит DS, значит читабельность кода упала.
И зачем массивы в константах?

[zeuss56]

Stalker_RED:
>Я вот тоже не понимаю зачем вы их линкуете
Во многих языках если не нашлось переменной с опеределённым именем определённой в текущем пространстве переменных, она ищется выше. В PHP такого нет. Если я обращаюсь к `$anyvar` из метода класса, то он не найдёт переменную и выкинет нотис. А я хочу обращаться к `$anyvar` класса. Вот и переименовываю `$this->anyvar` в `$anyvar` чтобы меньше писать `$this->` внутри метода. А ссылка - чтобы и чтение и запись в переменную работала. Я и не думал, что это стоит объяснения.
>Точно также непонятно, зачем вы делаете штуки типа `const DS = DIRECTORY_SEPARATOR`
Вам не кажется слишком широким `DIRECTORY_SEPARATOR` ? Довольно часто видел такие переименовывания в разном коде и по этому поводу были споры. И `const N = "\n";` тоже неплохая константа. А когда-то давно, во времена, когда HTML совался прямо в код, для него был `define('BR', '
');`, тогда ещё был только `define()` :)
>И зачем массивы в константах?
Списки тоже бывают константными и их нужно хранить. Не даром же в PHP7 это реализовано нормальным способом без `serialize()` / `unserialize()`.

[zeuss56]

Stalker_RED , Антон :
Может посоветуете книги?

[Stalker_RED]

Вадим Егоров: Ага, яснопонятно, стандартные названия констант слишком широкие, значит стоит выдумать свои


Вы, конечно, можете писать как угодно и забить на все PSR-ы, style guide'ы и прочую ерундень, но только до тех пор, пока вы пишете для себя лично и никто не делает вам код ревью. Сложновато будет объяснить тимлиду, что это все сделано для удобства.

На счет книг - в верхней части этой страницы есть поиск. Напишите там "книги php" или "книги php для начинающего". В соседнем вопросе котерова рекомендовали вроде. Свежее издание.

[zeuss56]

Stalker_RED: Спасибо. Поиск как поиск, а я предполагал, вы предложите что-то для моего случая. Котерова почитаю, OnYourLips предложил перед этим изучить пару фреймворков.