Оправдано ли увеличение и дублирование кода для разбиения логических процессов?
Доброго времени суток!
Пишу приложение для работы с посылками.
Изначально планировалось использовать статусы посылок для отслеживания её состояния и в зависимости от текущего статуса посылки разрешать определённые действия над ней.
Но решил отказаться от данного метода, так как при изменении конфигурации статусов изменяется и логика поведения приложения и может возникнуть путаница и нарушение логики бизнес процесса.
Было принято решение отслеживать состояние посылки без использования статусов, . Например: в БД имеется 3 таблицы, Отгрузки, Посылки и Возвраты. Для того что бы найти посылки которые готовы к отгрузке но еще не отгружены, достаточно заглянуть в таблицу Отгрузки и найти все посылки у которых не присвоен номер отгрузки или дата отгрузки, так же обстоят дела и с Возвратами.
Для того что бы найти посылки полученные на складе и готовые к выдаче клиенту надо извлечь из таблицы Посылки, данные о посылках у которых есть номер отгрузки и нет признака продажи и нет номера возврата. Статусы посылок по прежнему есть, но теперь они используются для хранения истории и в случае нарушения конфигурации статусов максимум что может произойти, это записаться в историю неверный статус, а с посылкой произойдёт то, что должно было произойти.
А теперь к самой сути вопроса.
Приняв решение перейти от статусов к процессам, я намерено увеличил код и в некоторых местах даже имеются похожие запросы к БД, кроме того при использовании статусов мне надо было изменять данные только в одной таблице, а теперь в двух, как вы считаете, оправданы ли такие изменения?
По сути к конфигурации статусов никто кроме меня и не должен был лезть, но я побоялся, что спустя какое то время я или кто то залезет в код и может что то изменить в конфигурации статусов таким образом нарушив логику работы приложения. Теперь для того что бы что то нарушить, нужно изрядно потрудится и изменить код конкретного процесса.
sim3x, за основу я взял ядро одной из cms, ядро очень похоже на codeignitier.
По поводу тестов не совсем понятен вопрос. Как можно писать приложение не тестируя его, или вы имеете ввиду какие то определённые тесты? В рефакторинге я совсем профан, поэтому и обратился с таким вопросом к сообществу.
Наверное нужно исходить из того будут ли меняться процессы. Если будут, то ваш вариант сложнее дорабатывать. Если нет, то нужно наметить точки расширения в вашем коде и понять насколько трудно будет вносить изменения так, чтобы не поломать уже работающий код. Как можно будет масштабировать ваше решение, если нагрузка сильно возрастёт. Кроме этого, если требуется по процессу менять синхронно несколько таблиц, значит в целях обеспечения целостности данных нужно такие изменения делать атомарно, т.е. в транзакции.
А добавление транзакции может привести к блокировкам и дедлокам при многопользовательской работе, если конечно количество операций будет большим. В общем чтобы понять плюсы и минусы нужно понять как именно может потребоваться доработать ваше приложение и прикинуть профиль нагрузки.
Те процессы которые уже существуют скорее всего изменятся не будут. В дальнейшем скорее всего будет несколько новых процессов которые практически не будут связаны с уже существующими, а значит и логика работы у них будет своя. Так как я самоучка то про транзакции только что первый раз услышал. Правильно ли я понял, что транзакция не даст произойти запросу в БД если не выполнен предыдущий запрос? Скорее всего транзакции не нужны, так как в каждой таблице имеются определённые признаки позволяющие точно определять нужно ли внести изменения. Приведу пример функции и её запросов на отгрузку посылки и приём посылок на складе, думаю станет более понятно.
Отгрузка посылок:
public function addShipment($company_id, $point_id, $parcels) {
$this->db->query("INSERT INTO " . DB_PREFIX . "shipment SET company_id = '" . (int)$company_id . "', point_id = '" . (int)$point_id . "', quantity = '" . (int)count($parcels) . "', user = '" . $this->db->escape($this->admin->getFullname()) . "', date_added = NOW()");
$shipment_id = $this->db->getLastId();
// На момент этого запроса дынные о посылке уже присутствуют в базе данных и остаётся её только отгрузить присвоив номер перевозки shipment_id
$this->db->query("UPDATE " . DB_PREFIX . "shipment_parcel SET shipment_id = '" . (int)$shipment_id . "', date_modified = NOW() WHERE parcel_id IN (" . implode(',', $parcels) . ")");
//Изменяем состояние посылки
$this->db->query("UPDATE " . DB_PREFIX . "parcel SET shipment_id = '" . (int)$shipment_id . "', date_modified = NOW() WHERE parcel_id IN (" . implode(',', $parcels) . ")");
// Записываем статус в историю
foreach($parcels as $parcel_id) {
$this->db->query("INSERT INTO " . DB_PREFIX . "parcel_history SET parcel_id = '" . (int)$parcel_id . "', parcel_status_id = '" . (int)$this->config->get('config_expected_status') . "', user = '" . $this->db->escape($this->admin->getFullname()) . "', date_added = NOW()");
}
}
Приём посылок на складе:
public function updateShipment($parcels) {
$this->db->query("UPDATE " . DB_PREFIX . "shipment_parcel SET date_delivery = NOW() WHERE point_id = '" . (int)$this->session->data['point_id'] . "' AND parcel_id IN (" . implode(',', $parcels) . ")");
$this->db->query("UPDATE " . DB_PREFIX . "parcel SET date_return = '" . $this->db->escape(date('Y-m-d', strtotime('+' . $this->config->get('config_parcel_deadline') .' days'))) . "', date_modified = NOW() WHERE point_id = '" . (int)$this->session->data['point_id'] . "' AND parcel_id IN (" . implode(',', $parcels) . ")");
foreach($parcels as $parcel_id) {
$this->db->query("INSERT INTO " . DB_PREFIX . "parcel_history SET parcel_id = '" . (int)$parcel_id . "', parcel_status_id = '" . (int)$this->config->get('config_recieved_status') . "', user = '" . $this->db->escape($this->operator->getFullname()) . "', date_added = NOW(), date_modified = NOW()");
}
}
Я разбил все процессы на 5 этапов, Отгрузка, Приёмка, Исполнение, Отмена, Возврат. Все процессы зависимы от предыдущего процесса, то-есть пока не произойдёт например Отгрузка, принимать нечего будет, так как выборка будет пуста.
Таким образом для каждого процесса создаются практически идентичные функции и запросы, в которых только изменяется название функций, таблиц и статусов. Вот я и хочу понять, насколько логично использование такого подхода и оправдывает ли это увеличение моего кода, ведь по сути я мог использовать 1 таблицу для изменения статуса посылки и на основании этого принимать решение о дальнейшей её судьбе, но в таком случае нарушить работу логики достаточно было бы изменив например в конфигурации статусов статус Получена на статус Ожидает отгрузки.
По поводу нагрузки сам ещё не имею представления так как всё будет зависеть от количества клиентов и количества посылок которые они будут отправлять. Процесс будет происходить следующим образом, клиенты регистрируют посылки, после чего их отгружают на склад, на складе, повторно обрабатывают посылки, тоесть принимают, дальше либо выдают получателю, либо отменяют и формируют возвраты.
В приведённом вами коде в первой функции есть четыре запроса. Если не выполнять их в транзакции, то возможно что например первый insert отработал, а затем случился разрыв коннекта с базой и в итоге посылка зависла в непонятном состоянии. Если весь пакет из этих запросов обернуть в транзакцию, то в случае любого сбоя в любой момент выполнения пакета все, что было выполнено до момента сбоя будет возвращено в исходное состояние, т.е. в состояние до начала выполнения первого запроса пакета. Если же все ОК, то транзакция фиксируется, т.е. все изменения вступают в силу.
То что все состояния выделены в отдельные процедуры это хорошо, пусть есть дублирование, но зато код не зависим и доработав один процесс можно не опасаться регресса в других(но тесты для каждого процесса нужно написать). То что вы заложили таблицу истории в проект это тоже хорошо, аудит очень нужен, для разбора конфликтных ситуаций. Я бы заложил в проект механизм чистки или архивации для тех посылок, которые уже доставлены, например завести рядом таблицы с аналогичной структурой и постфиксом arch и переливать туда данные по посылкам, которые доставлены например месяц назад или ранее(можно сделать настраиваемым). Таким образом, оперативные таблицы не будут сильно распухать.
LaRN, как хорошо, что именно вы мне написали ответ. Спасибо большое! Я sql изучал читая чужой код и время от времени заглядывая в google и не знал о существовании транзакций, точнее догадывался, что должен быть какой-то похожий механизм, но отыскать как то не смог, приходилось изворачиваться всякими JOINами и подзапросами в некоторых местах, что очень сильно мучало мою совесть.
На счёт архивных таблиц тоже задумывался, только думал, что неправильно будет создавать аналогичные таблицы, но похоже, что я ошибался и в некоторых случаях похоже дублирование неизбежно.
Если вас не затруднит, сможете, пожалуйста, написать пример какого нибудь моего запроса с транзакциями.
И хотелось бы подробней разузнать про:
добавление транзакции может привести к блокировкам и дедлокам при многопользовательской работе
Т. е. получается, что при большом количестве пользователей будет происходить, что то типа постановка запросов в очередь для каждого пользователя?
Чтобы открыть транзакцию нужно вызвать метод begin_transaction() у объекта db
Чтобы зафиксировать транзакцию нужно вызвать метод commit()
Т.е. Для вашего кода д.б.что-то вроде такого:
public function updateShipment($parcels) {
$this->db->begin_transaction()
$this->db->query("UPDATE " . DB_PREFIX . "shipment_parcel SET date_delivery = NOW() WHERE point_id = '" . (int)$this->session->data['point_id'] . "' AND parcel_id IN (" . implode(',', $parcels) . ")");
$this->db->query("UPDATE " . DB_PREFIX . "parcel SET date_return = '" . $this->db->escape(date('Y-m-d', strtotime('+' . $this->config->get('config_parcel_deadline') .' days'))) . "', date_modified = NOW() WHERE point_id = '" . (int)$this->session->data['point_id'] . "' AND parcel_id IN (" . implode(',', $parcels) . ")");
foreach($parcels as $parcel_id) {
$this->db->query("INSERT INTO " . DB_PREFIX . "parcel_history SET parcel_id = '" . (int)$parcel_id . "', parcel_status_id = '" . (int)$this->config->get('config_recieved_status') . "', user = '" . $this->db->escape($this->operator->getFullname()) . "', date_added = NOW(), date_modified = NOW()");
И да, использование транзакций может приводить к блокировкам, в течении того времени пока транзакция не зафиксирована или не отменена. Это значит, что изменные или добавленные в одном коннекте в транзакции записи будут не доступны в запросах из другого коннекта и запросы будут висет в ожидании того, что завершится наложившая блокировку транзакция. Но такое поведение можно скорректировать если указать уровень изоляции. Про это тоже можно почитать в указанных ссылках.
Средний
