# 🔍 Анализ алгоритма разделения АСУ ТП простоя --- ## ✅ Что реализовано корректно Прежде чем перейти к замечаниям, отмечу сильные стороны алгоритма: - **Шаг 1** — поиск именно копии (APCS-записи) по `Идентификатор родительского простоя` с проверкой на отсутствие и на дубликаты — логически верен ✔️ - **Шаги 2–3** — последовательная проверка дедлайна и наличия даты окончания. Логика корректна: если запись «открытая» (End = null), то дедлайн не применяется (шаг 2 пропускает), но шаг 3 всё равно блокирует разделение ✔️ - **Шаг 4** — валидация `End > Start` для каждого элемента — стандартная и правильная ✔️ - **Шаг 5** — проверка пересечений между элементами с полуоткрытым интервалом `[Start; End)` — верно ✔️ - **Шаг 8a.iv** — формула пересечения `(Start_existing < End_new) AND (End_existing IS NULL OR End_existing > Start_new)` — математически корректна для полуоткрытых интервалов ✔️ - **Шаг 10** — откат транзакции при ошибке — абсолютно необходимый элемент ✔️ --- ## 🔴 Критические / существенные замечания ### 1. 📌 Отсутствие сортировки перед шагом 7 **Проблема:** Шаг 7 выполняет проверку непрерывности, обходя список **по индексам** (`Dt[i].Start == Dt[i-1].End`). Это предполагает, что входной список **уже отсортирован** по `Start`, но **нигде это явно не гарантируется**. **Пример сбоя:** Пусть список пришёл в порядке: | Индекс | Start | End | |--------|-------|-----| | 0 | 10:00 | 11:00 | | 1 | 12:00 | 13:00 | | 2 | 11:00 | 12:00 | - Шаг 5 пройдёт ✅ (пересечений нет) - Шаг 7 проверит: `Dt[2].Start (11:00) != Dt[1].End (13:00)` → ❌ **ложная ошибка**, хотя интервалы непрерывны **Рекомендация:** Добавить перед шагом 7 (или даже перед шагом 4) явный шаг: > **6.5.** Отсортировать коллекцию «Список простоев» по возрастанию поля «Дата и время начала». --- ### 2. 📌 Порядок шагов: модификация данных (шаг 6) до завершения валидации (шаг 7) **Проблема:** Текущий порядок: ``` Шаги 1–5 → валидация ✅ Шаг 6 → УДАЛЕНИЕ старых записей 🗑️ ← модификация! Шаг 7 → валидация непрерывности ✅ Шаг 8a → валидация пересечений ✅ Шаг 8b → создание записей ✏️ ← модификация! Шаг 9 → обновление родителя ✏️ ← модификация! ``` Удаление происходит **до** проверки непрерывности (шаг 7). Если шаг 7 упадёт с ошибкой, придётся откатывать удалённые записи. Это работает благодаря шагу 10, но **противоречит принципу** «сначала валидация, потом модификация». **Дополнительно:** шаг 8a (проверка пересечений) зависит от шага 6 (удаление старых детей), потому что старые `APCSManual`-записи того же родителя попадут в выборку и дадут ложное пересечение. **Рекомендация:** Модифицировать шаг 8a, добавив исключение: > **8a.iii (дополнение):** Также исключить из выборки записи, у которых `Идентификатор родительского простоя` = полю «Идентификатор исходного простоя» из входа функции **И** `Создание записи` = `"APCSManual"`. Это позволит **перенести шаг 6 после всех валидаций**: ``` Шаги 1–5, 7 → вся валидация Шаг 8a (для ВСЕХ элементов) → валидация пересечений ─── граница: далее только модификации ─── Шаг 6 → удаление старых записей Шаг 8b → создание новых записей Шаг 9 → обновление родителя ``` --- ### 3. 📌 Вновь созданные записи (8b) попадают в проверку пересечений (8a) на следующих итерациях **Проблема:** Шаг 8 итерирует по элементам: для каждого `Dt[i]` сначала 8a (валидация), потом 8b (создание). При обработке `Dt[1]` запись для `Dt[0]` **уже создана в БД** и попадёт в выборку шага 8a. **Почему сейчас не ломается:** Благодаря **строгому неравенству** `>` в условии 8a.iv: - `Dt[0].End = Dt[1].Start` (непрерывность из шага 7) - Условие: `End_existing > Start_new` → `Dt[0].End > Dt[1].Start` → **false** ✅ **Но это хрупко:** логика корректности зависит от тонкого совпадения строгого неравенства и непрерывности интервалов. Любое изменение в будущем может сломать это поведение. **Рекомендация:** Либо (A) разделить шаг 8 на два прохода (сначала 8a для всех, потом 8b для всех), либо (B) добавить в 8a.iii исключение записей с тем же `Идентификатор родительского простоя` и `Создание записи` = `"APCSManual"` (как в рекомендации к п.2 выше — это решает обе проблемы). --- ## 🟡 Важные замечания ### 4. ⚠️ Нет проверки на минимальное количество элементов в `DowntimesList` **Проблема:** Алгоритм не проверяет, что список непустой. При **пустом списке**: - Шаг 7 — ничего не проверяет (условие `length-1 до 1` не выполняется при `length=0`) - Шаг 8 — ни одной итерации, записи не создаются - Шаг 9a — не находит `APCSManual`-записей → обращение к «первой» и «последней» записи пустого набора → **ошибка / undefined behavior** 💥 Также при **одном элементе** происходит не «разделение», а фактически «замена» — это может быть как ожидаемым поведением, так и багом. **Рекомендация:** Добавить проверку (например, между шагами 3 и 4): > **3.5.** Если коллекция «Список простоев» содержит менее 2 элементов, вывести ошибку: *«Для разделения простоя необходимо указать минимум 2 новых части»*. *(Или минимум 1, если «замена» одним элементом — допустимый сценарий)* --- ### 5. ⚠️ Шаг 8a не остановит выполнение при пересечении с «полуоткрытыми» записями (сомнительный момент) В шаге 8a.ii вы сами пометили: *«Сомнительный момент — возможно лучше не учитывать все статусы или не учитывать полуоткрытые записи (без даты окончания)»*. **Суть:** Запись с `End = null` (открытый простой) по условию 8a.iv **всегда** даст пересечение (условие `End = null` попадает в ветку `OR`). Это значит, что если на агрегате есть **любой незакрытый простой**, то разделение другого простоя будет **заблокировано**, даже если интервалы логически не конфликтуют. **Рекомендация:** Обсудить с заказчиком — блокировка при наличии открытых простоев может быть слишком строгой. Возможные варианты: - Исключить записи с `End = null` из проверки - Исключить записи с определёнными статусами - Оставить текущее поведение, но с понятным сообщением об ошибке --- ### 6. ⚠️ Потенциальное «двойное покрытие» после шага 9b **Проблема:** После выполнения алгоритма в БД существуют: - **Родительская** APCS-запись с `[first_child.Start, last_child.End]` - **Дочерние** APCSManual-записи, покрывающие тот же период Если другие функции системы делают выборку **всех** простоев по оборудованию (без фильтрации по иерархии), произойдёт **двойной учёт** времени простоя. **Рекомендация:** Убедиться, что другие модули системы при расчёте длительности простоев: - Либо берут **только дочерние** записи (если они есть) - Либо фильтруют по `Создание записи` ≠ `"APCS"` при наличии `APCSManual`-детей - Либо используют флаг «разделена» на родительской записи --- ## 🟢 Мелкие замечания и предложения ### 7. 💡 Явное указание на прерывание выполнения при ошибке В шагах 8a.A формулировка «вывести ошибку» не содержит явной инструкции **прекратить выполнение**. Хотя это подразумевается (шаг 10 говорит про откат), рекомендую добавить: > *«…вывести ошибку и **прекратить выполнение функции** (перейти к шагу 10)»* --- ### 8. 💡 Отсутствие валидации ссылочных данных Алгоритм не проверяет существование: - `TypeId` (Идентификатор типа простоя) - `KindId` (Идентификатор вида простоя) - `ReasonCode` (Код причины простоя) - `ServiceId` (Идентификатор службы) - `SectionCode` (Код зоны агрегата) Это может обрабатываться на уровне БД (FK constraints) или на уровне API, но если это не так — стоит добавить валидацию. --- ### 9. 💡 Формула дедлайна (шаг 8b.xiv) — уточнение поведения при граничных случаях Формула: `{Текущая дата + 1 день, время = 10:00}`. **Вопрос:** Что если текущее время **до 10:00**? Например: - Текущее время: `2025-11-11 08:00` - Дедлайн: `2025-11-12 10:00` (через 26 часов) А если: - Текущее время: `2025-11-11 23:00` - Дедлайн: `2025-11-12 10:00` (через 11 часов) Оба случая дают одну и ту же дату дедлайна, но разный «запас времени». Возможно, это и есть задуманное поведение, но стоит подтвердить. --- ## 📋 Итоговая сводка | # | Категория | Описание | Критичность | |---|-----------|----------|-------------| | 1 | Логика | Нет сортировки списка перед проверкой непрерывности (шаг 7) | 🔴 | | 2 | Архитектура | Модификация данных (шаг 6) до завершения всех валидаций | 🔴 | | 3 | Хрупкость | Созданные записи попадают в overlap-check следующих итераций | 🔴 | | 4 | Валидация | Нет проверки минимального количества элементов | 🟡 | | 5 | Бизнес-логика | Полуоткрытые записи блокируют разделение | 🟡 | | 6 | Дизайн | Двойное покрытие (родитель + дети) | 🟡 | | 7 | Документация | Нет явного указания на прерывание при ошибке | 🟢 | | 8 | Валидация | Нет проверки ссылочных данных | 🟢 | | 9 | Уточнение | Граничные случаи формулы дедлайна | 🟢 | --- ## 🛠️ Рекомендуемый порядок шагов (после исправлений) ``` 1. Найти родительскую APCS-запись 2. Проверить дедлайн 3. Проверить наличие даты окончания 3.5 Проверить минимальное кол-во элементов (≥ 2) 4. Проверить End > Start у каждого элемента 4.5 Отсортировать список по Start ↑ 5. Проверить отсутствие пересечений между элементами 7. Проверить непрерывность интервалов 8a. Проверить пересечения с существующими записями (для ВСЕХ элементов, исключая родителя И старые APCSManual-записи того же родителя) ─── все валидации пройдены ─── 6. Удалить старые APCSManual-записи 8b. Создать новые записи 9. Обновить родительскую запись 10. Откат при ошибке на шагах 6–9 ```