Pull to refresh

Как из двух строчек кода сделать 200, и почему так делать надо

Reading time 7 min
Views 13K

Всем привет.


Я (мы как фирма) допиливаем платформу от поставщика, платформа это плагин для WordPress. С фронтендом, JS, HTML я знаком лишь постольку поскольку, поэтому приведённые решения могут оказаться не грамотными, но суть статьи не в этом.


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


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


Введение


Читать статью не читая коменты в коде, не имеет смысла, потому что коменты кода не продублированы в тексте статьи и текст статьи подразумевает что читатель с коментами ознакомился.


Задача


Необходимо было внести изменения в JS, изменения я вношу в отдельные файлы, что бы как можно меньше "загрязнять" исходный код поставщика платформы, так в будущем будет проще адаптировать обновления исходников платформы.


Соответственно код нового функционала находиться в новых файлах которые надо подгружать на страничке.


Задача: требуется при загрузке страницы подгружать определённые файлы.


Первый блин комом


+<script src="e-cryptex.js"></script>
+<script src="wp-content/themes/tol-child/js/child_realforex.js"></script>

(строчки с "+" это новые строки в исходном коде)


Решение было протестировано для главной страницы и там оно работало, позже выяснилось что для страниц с адресом example.com/about, решение не работало, потому что браузер пытался подгрузить example.com/about/e-cryptex.js и браузер получал — ошибку 404, потому что файл реально лежал в корне — example.com/e-cryptex.js .


Второй вариант


Посмотрел как файлы к страничке цепляют разработчики платформы:


wp_register_script(FILE-ID,FILE-PATH);
wp_enqueue_script(FILE-ID);

Файлы прикрепляются с использованием функционала WordPress — ок, сделаем так же:


// Было:

-<script src="e-cryptex.js"></script>
-<script src="wp-content/themes/tol-child/js/child_realforex.js"></script>

// Стало :

+<?php
+const E_CRYPTEX_JS ='e-cryptex_js';
+wp_register_script(E_CRYPTEX_JS,'/e-cryptex.js',array(),null);
+wp_enqueue_script(E_CRYPTEX_JS);
+const CHILD_REALFOREX_JS = 'child_realforex_js';
+wp_register_script(
+        CHILD_REALFOREX_JS,
+        '/wp-content/themes/tol-child/js/child_realforex.js',
+        array(E_CRYPTEX_JS),
+        null);
+wp_enqueue_script(CHILD_REALFOREX_JS);
+?>

(строчки с "+" это новые строки в исходном коде, строчки с "-" это строки удалённые из исходного кода).


Проверил — работает, ок.


Было две строчки — стало 12, всё в пределах одного файла.


Рефакторинг


Посмотрел я на этот код, и моё чувство прекрасного взбунтовалось:


  • человек не знакомый с WordPress ни чего не поймёт — придётся лезть в справочник что бы понять назначение функций wp_register_script() и wp_enqueue_script()
  • явно делается одно и то же два раза подряд, аргументы только разные — нарушается DRY

Ок, рефакторим.


Во первых, делаем класс (один файл), во вторых, класс подключаем и используем (изменяем другой файл).


Делаем класс


+class Migesco {
+
+    const SCRIPT = 'script';
+    const SOURCE = 'source';
+    const DEPENDENCY = 'dependency';
+

// На всех страницах, где был изменён функционал используется 'configuration.js', 
// поэтому для него делаем "жёсткий" вариант подгрузки файла.

+    static function attachConfigurationJS(){
+        $configurationFiles = array(
+            array(Migesco::SCRIPT => 'configuration.js',
+                Migesco::SOURCE=>'/configuration.js'));
+        Migesco::includeScriptFiles($configurationFiles);
+    }

// На вход получаем массив с параметрами файлов и все их по очереди подключаем

+    static function includeScriptFiles($scriptFiles){
+        foreach ($scriptFiles as $scriptFile){
+            $dependency = array_key_exists(self::DEPENDENCY,$scriptFiles)
+                ? $scriptFile[self::DEPENDENCY]
+                : array();
+            self::includeScript($scriptFile[self::SCRIPT],$scriptFile[self::SOURCE],$dependency);
+        }
+    }

// Собственно подключение одного скрипта, 
// реализация скрыта от "пользователя", все как Ленин завещал 
// (метод надо было ещё приватным сделать)

+    static function includeScript($id,$source,$dependency){
+        wp_register_script($id,$source,$dependency,null);
+        wp_enqueue_script($id);
+    }
+}

Подключаем и используем


<?php
//Было:

-const E_CRYPTEX_JS ='e-cryptex_js';
-wp_register_script(E_CRYPTEX_JS,'/e-cryptex.js',array(),null);
-wp_enqueue_script(E_CRYPTEX_JS);
-const CHILD_REALFOREX_JS = 'child_realforex_js';
-wp_register_script(
-        CHILD_REALFOREX_JS,
-        '/wp-content/themes/tol-child/js/child_realforex.js',
-        array(E_CRYPTEX_JS),
-        null);
-wp_enqueue_script(CHILD_REALFOREX_JS);

// Стало:
// Подключаем 

+require_once(ABSPATH . 'configuration.php');

// Используем

+const ECRYPTEX_JS = 'cryptex';
+const TRADEROOM_SCRIPT_FILES = array(
+    array(Migesco::SCRIPT => ECRYPTEX_JS,
+        Migesco::SOURCE=>'/e-cryptex.js'),
+    array(Migesco::SCRIPT => 'child_realforex',
+        Migesco::SOURCE=>'/wp-content/themes/tol-child/js/child_realforex.js',
+        Migesco::DEPENDENCY =>ECRYPTEX_JS)
+);
+Migesco::includeScriptFiles(TRADEROOM_SCRIPT_FILES);
?>

Было 12 строчек исходников в одном файле стало 35, и в двух (поддерживай это потом, ищи рыщи где бы что бы ещё подправить, что бы ни чего не забыть, не пропустить, не упустить).


И ещё раз рефактринг


Посмотрел на новый код:


  • какие то статические методы,
  • какие то константы…

топорно смотрится! Помниться такой совет: "если у конструктора нет аргументов, то нужен ли такой класс ?"


Давайте переделаем, но нормальный класс, с нормальным конструктором и методами.


Класс


// Было:

-class Migesco {
-
-    const SCRIPT = 'script';
-    const SOURCE = 'source';
-    const DEPENDENCY = 'dependency';
-
-    static function attachConfigurationJS(){
-        $configurationFiles = array(
-            array(Migesco::SCRIPT => 'configuration.js',
-                Migesco::SOURCE=>'/configuration.js'));
-        Migesco::includeScriptFiles($configurationFiles);
-    }
-    static function includeScriptFiles($scriptFiles){
-        foreach ($scriptFiles as $scriptFile){
-            $dependency = array_key_exists(self::DEPENDENCY,$scriptFiles)
-                ? $scriptFile[self::DEPENDENCY]
-                : array();
-            self::includeScript($scriptFile[self::SCRIPT],$scriptFile[self::SOURCE],$dependency);
-        }
-    }
-    static function includeScript($id,$source,$dependency){
-        wp_register_script($id,$source,$dependency,null);
-        wp_enqueue_script($id);
-    }
-}

//Стало:
// Один класс пришлось разделить на три:
// Один управляет
// Другой выполняет
// Третий данные хранит
//Для объединения классов делаем отдельное пространство имён

+namespace Migesco;
+
+

// Класс Управляет прикреплением файлов к html-страничке

+class Configurator
+{

// Тот самый жёсткий метод для привязки файла общего для всех страниц с изменённым функционалом

+    static function attachConfigurationJS()
+    {
+        $configurationFiles = array(
+            (new WebResource('configuration.js'))->setSource('/configuration.js'));
+        self::attachFiles($configurationFiles);
+    }
+

// Метод для прикрепления файла к html-страничке

+    static function attachFiles($resourceList)
+    {
+        (new Registrar($resourceList))->toRegistrate();
+    }
+}
+

// Класс Выполняет прикрепление файла к html-страничке

+class Registrar
+{

// Массив файлов для прикрепления

+    public $list = array();

// это свойство должно быть private (задел на будущий рефакторинг)

+    /** @var WebResource $resource */
+    public $resource = null;
+
+    public function __construct($list)
+    {
+        $isArray = is_array($list);
+        if ($isArray) {
+            $this->list = $list;
+        }
+    }
+

// Метод для регистрации файла в WordPress

+    function registerScript()
+    {
+        wp_register_script(
+            $this->resource->getName(),
+            $this->resource->getSource(),
+            $this->resource->getDependency(),
+            null);
+    }
+

// Метод для прикрепления файла

+    function enqueueScript()
+    {
+        wp_enqueue_script($this->resource->getName());
+    }
+

// Метод для прикрепления списка файлов

+    function toRegistrate()
+    {
+        $result = false;
+        foreach ($this->list as $resource) {
+            /** @var WebResource $resource */
+            $isResource = $resource instanceof WebResource;
+            if ($isResource) {
+                $this->resource = $resource;
+                $this->registerScript();
+                $this->enqueueScript();
+
+                $result = true;
+            }
+        }
+        return $result;
+    }
+}
+

// Класс для Хранения данных

+class WebResource
+{

// Путь к файлу

+    public $source = '';

// Идентификатор файла

+    public $name = '';

// Зависимости файла

+    public $dependency = array();
+
+    public function __construct($name)
+    {
+        $this->setName($name);
+    }
+
+    /**
+     * @param string $source
+     * @return WebResource
+     */
+    public function setSource($source)
+    {
+        $this->source = strval($source);
+        return $this;
+    }
+
+    /**
+     * @param string $name
+     * @return WebResource
+     */
+    public function setName($name)
+    {
+        $this->name = strval($name);
+        return $this;
+    }
+
+    /**
+     * @param array $dependency
+     * @return WebResource
+     */
+    public function setDependency($dependency)
+    {
+        $isArray = is_array($dependency);
+        if ($isArray) {
+            $this->dependency = $dependency;
+        }
+        return $this;
+    }
+
+    /**
+     * @return string
+     */
+    public function getSource()
+    {
+        return $this->source;
+    }
+
+    /**
+     * @return string
+     */
+    public function getName()
+    {
+        return $this->name;
+    }
+
+    /**
+     * @return array
+     */
+    public function getDependency()
+    {
+        return $this->dependency;
+    }
+}

Использование


 <?php
// Было:

-const TRADEROOM_SCRIPT_FILES = array(
-    array(Migesco::SCRIPT => ECRYPTEX_JS,
-        Migesco::SOURCE=>'/e-cryptex.js'),
-    array(Migesco::SCRIPT => 'child_realforex',
-        Migesco::SOURCE=>'/wp-content/themes/tol-child/js/child_realforex.js',
-        Migesco::DEPENDENCY =>ECRYPTEX_JS)
-);
-Migesco::includeScriptFiles(TRADEROOM_SCRIPT_FILES);

// Стало:

+$traderoomScriptFiles = array(
+    (new Migesco\WebResource(ECRYPTEX_JS))
+        ->setSource('/e-cryptex.js'),
+    (new Migesco\WebResource('child_realforex'))
+        ->setSource('/wp-content/themes/tol-child/js/child_realforex.js')
+        ->setDependency(array(ECRYPTEX_JS))
+);
+Migesco\Configurator::attachFiles($traderoomScriptFiles);
 ?>

Можно ещё раз сделать рефакторинг(платформа работает на PHP 5.6 — с удовольствием бы везде расставил типы, но к сожалению нельзя, можно статичный класс Configurator, сделать более "человеческим", например, инициализировать от списка файлов — веб-ресурсов и метод attachFiles сделать не статическим), но идеальные вещи для идеального мира, мы живём в реальном, и это вносит свои коррективы — время на задачу потрачено — работа по задаче прекращена.


Итого: было 35 строк в исходниках в двух файлах, стало ~170, также в двух файлах.


Что мы получили ?


  1. Теперь не надо лезть в хэлп WordPress`а, что бы понять назначение параметров "функций".
  2. Теперь у нас есть обёртка для подключения файлов и мы можем без болезненно изменять алгоритм подключения файлов — не придётся править все вызовы wp_register_script и wp_enqueue_script
  3. Теперь мы можем сменить WordPress на что угодно и переписать придётся только класс
    Registrar, не надо заменять все вызовы wp_register_script и wp_enqueue_script и следовать логике WordPress`а.

Стоило оно того?


У каждого свой подход к поддержке кодовой базы и своё мнение.


Мой ответ — да.

Only registered users can participate in poll. Log in, please.
Стоило оно того?
4.66% Да, стоило 11
63.14% Нет, не стоило 149
32.2% Я мимо проходил 76
236 users voted. 55 users abstained.
Tags:
Hubs:
-47
Comments 74
Comments Comments 74

Articles