// Формируем движения по партиям для возврата
Движения = РаспределитьПоПартиям(-Количество, ПараметрыРаспределения);
Этот код выглядит правильно. Возврат товара -- количество со знаком минус, передаём в функцию распределения по партиям. Логично. Очевидно. Работает.
Работает ровно до тех пор, пока не оформляется частичный возврат по оплаченному заказу. Тогда вместо возврата получается приход. Резерв не снимается, а удваивается. Остатки на складе растут вместо того, чтобы уменьшаться. И бухгалтерия замечает это через месяц, когда начинает сводить оборотку.
Код, который выглядит правильно
Розничная сеть с интернет-магазином. Заказы принимаются на сайте, обрабатываются в 1С. При возврате товара создаётся документ возврата, который делает обратные движения: снимает резервы, уменьшает остатки, корректирует взаиморасчёты.
Модуль проведения документа возврата вызывает функцию распределения по партиям. Логика такая: нужно понять, из каких партий были отгружены товары, и сделать обратные движения именно по этим партиям. Не просто "минус 3 штуки откуда-нибудь", а "минус 2 штуки из партии от 15.01, минус 1 штука из партии от 22.01".
Код вызова:
Процедура СформироватьДвиженияВозврата(Движения, ТаблицаТоваров, ПараметрыПроведения)
Для Каждого СтрокаТовара Из ТаблицаТоваров Цикл
ПараметрыРаспределения = Новый Структура;
ПараметрыРаспределения.Вставить("Номенклатура", СтрокаТовара.Номенклатура);
ПараметрыРаспределения.Вставить("Склад", СтрокаТовара.Склад);
ПараметрыРаспределения.Вставить("Заказ", ПараметрыПроведения.Заказ);
// Возврат -- значит минус
КоличествоВозврата = -СтрокаТовара.Количество;
РезультатРаспределения = РаспределитьПоПартиям(
КоличествоВозврата, ПараметрыРаспределения);
Для Каждого СтрокаПартии Из РезультатРаспределения Цикл
Движение = Движения.РезервыТоваров.Добавить();
Движение.ВидДвижения = ВидДвиженияНакопления.Расход;
Движение.Номенклатура = СтрокаТовара.Номенклатура;
Движение.Количество = СтрокаПартии.Количество;
// ...
КонецЦикла;
КонецЦикла;
КонецПроцедуры
Посмотрите на строку КоличествоВозврата = -СтрокаТовара.Количество. Количество в документе возврата -- положительное число (3 штуки). Минус перед ним делает его отрицательным (-3). Передаём в функцию распределения. Пока всё нормально.
А теперь загляну внутрь функции РаспределитьПоПартиям:
Функция РаспределитьПоПартиям(Количество, ПараметрыРаспределения)
// ... запрос остатков по партиям ...
ОсталосьРаспределить = -Количество; // <-- вот он
Результат = Новый ТаблицаЗначений;
Результат.Колонки.Добавить("Партия");
Результат.Колонки.Добавить("Количество");
Для Каждого СтрокаОстатков Из ВыборкаОстатков Цикл
Если ОсталосьРаспределить <= 0 Тогда
Прервать;
КонецЕсли;
РаспределяемоеКоличество = Мин(СтрокаОстатков.Количество, ОсталосьРаспределить);
НоваяСтрока = Результат.Добавить();
НоваяСтрока.Партия = СтрокаОстатков.Партия;
НоваяСтрока.Количество = РаспределяемоеКоличество;
ОсталосьРаспределить = ОсталосьРаспределить - РаспределяемоеКоличество;
КонецЦикла;
Возврат Результат;
КонецФункции
Видите? Строка ОсталосьРаспределить = -Количество. Функция тоже инвертирует знак. Она была написана в расчёте на то, что для возврата ей передадут отрицательное количество, и внутри она сделает его положительным, чтобы нормально работать с остатками. Разумная идея. Только вызывающий код уже сделал это за неё.
Что происходит по шагам:
- Количество в документе возврата: 3 (три штуки возвращаем)
- Вызывающий код:
КоличествоВозврата = -3 - Функция:
ОсталосьРаспределить = -(-3) = 3 - Функция распределяет 3 штуки по партиям и возвращает таблицу с положительными количествами
- Движения регистра создаются с положительными количествами
Минус на минус дал плюс. Возврат превратился в приход.
Почему проявился через месяц
Этот баг жил в коде с момента написания. Но никто его не замечал, потому что для его проявления нужно было совпадение трёх условий:
Условие 1: частичный возврат. Полный возврат заказа обрабатывался другой веткой кода -- через сторнирование всего документа целиком. Функция РаспределитьПоПартиям вызывалась только при частичном возврате -- когда из заказа на 10 позиций возвращают 2.
Условие 2: оплаченный заказ. Для неоплаченных заказов резервы не создавались, и движения по регистру РезервыТоваров не формировались. Баг был именно в движениях по резервам.
Условие 3: конкретная номенклатура с партионным учётом. Для товаров без партионного учёта функция распределения не вызывалась -- количество списывалось напрямую.
Три условия одновременно. Частичный возврат. Оплаченный заказ. Товар с партиями. В интернет-магазине это случалось нечасто. Клиенты обычно возвращали заказ целиком или вообще не возвращали. Частичный возврат оплаченного заказа с товаром, у которого ведётся партионный учёт, -- это был редкий сценарий.
Но через месяц таких случаев накопилось достаточно, чтобы бухгалтер увидел расхождение в оборотной ведомости. Резервы на складе были завышены. Не критически -- на несколько десятков позиций из тысяч. Но бухгалтер заметил.
Классика: чем реже баг проявляется, тем дольше он живёт и тем больше данных успевает испортить. Если бы ошибка была в полном возврате -- её бы заметили в первый день. Но она была в редком сценарии, и месяц тихо накапливала неправильные данные.
Минус на минус = плюс, но не в бухгалтерии
В математике двойная негация -- тождественное преобразование. Минус на минус даёт плюс, и все об этом знают. В программировании двойная негация -- почти всегда баг.
Проблема в том, что каждый "минус" в отдельности выглядит логично. Разработчик, который писал вызов функции, рассуждал: "Возврат -- значит отрицательное количество". Правильно. Разработчик, который писал функцию, рассуждал: "Мне передадут отрицательное число, но распределять нужно положительное -- инвертирую". Тоже правильно. Каждый из них принял разумное решение. Вместе эти два разумных решения дали баг.
Это частный случай более общей проблемы: размазанная ответственность за знак. Кто отвечает за то, что количество передаётся с правильным знаком -- вызывающий код или функция? Если оба -- двойная негация. Если никто -- количество без знака, когда нужен минус.
В математике есть понятие идемпотентности: повторное применение операции не меняет результат. Инвертирование знака -- не идемпотентная операция. Применённая дважды, она возвращает исходное значение. И код выглядит так, будто всё правильно -- числа положительные, движения создаются, документ проводится. Ошибка не на поверхности. Она в семантике: что означает это положительное число -- приход или возврат?
Похожая ситуация с неявными контрактами -- когда функция ожидает одно, а получает другое, и при этом не падает с ошибкой. Я разбирал этот паттерн в статье про API БСП: стандартный метод молча возвращал пустоту, потому что кастомный справочник нарушил неявный контракт.
Как найти подобное
Двойная негация -- баг, который не ловится стандартными средствами. Код компилируется. Тесты проходят (если тестируют типовой сценарий, а не edge case). Документы проводятся без ошибок. Движения создаются. Всё работает -- просто с неправильным знаком.
Вот подход, который я использовал для диагностики этого конкретного бага и который работает для похожих случаев.
Шаг 1: зафиксировать симптом. Бухгалтер сказал: "Резервы завышены". Это симптом, не диагноз. Первое, что я сделал -- выгрузил движения регистра РезервыТоваров за последний месяц и отфильтровал по документам возврата. Увидел: все движения возврата имеют положительный знак. Должны быть отрицательные (расход). Симптом подтверждён, направление поиска понятно.
Шаг 2: проследить цепочку формирования значения. Количество в движении -- откуда оно? Из таблицы распределения. Количество в таблице распределения -- откуда? Из функции РаспределитьПоПартиям. Что на входе функции? Что на выходе? Не отладчиком -- простым Сообщить() на каждом шаге. Вход: -3. Выход: таблица с количеством 3. Внутри функции знак поменялся. Нашёл строку с инверсией.
Шаг 3: понять контракт. Функция ожидает отрицательное число и инвертирует. Вызывающий код передаёт уже отрицательное. Двойная негация. Кто "виноват"? Зависит от контракта функции. Если в комментарии написано "Количество может быть положительным или отрицательным" -- виноват вызывающий код, не нужно было инвертировать. Если написано "Количество должно быть положительным, знак определяется контекстом" -- виновата функция, не нужно было инвертировать внутри.
В данном случае комментария не было. Контракт не задокументирован. Два разработчика сделали противоположные предположения о том, кто отвечает за знак.
Общие рекомендации для поиска подобных багов:
- Ищите парные инверсии. Если видите
-Количествов одном месте -- проверьте, нет ли инверсии дальше по цепочке. Grep по-Количествои-Колпо модулю. - Проверяйте edge cases вручную. Возьмите конкретный документ. Подставьте числа. Пройдите по коду с калькулятором. 3 --> -3 --> 3. Если знак вернулся к исходному -- проблема.
- Смотрите на движения регистров. Если документ возврата создаёт движения прихода -- что-то не так. Движения -- это конечный результат, по ним проще всего понять, что код делает на самом деле, а не что он должен делать.
- Тестируйте редкие сценарии. Полный возврат. Частичный возврат. Возврат одной позиции из десяти. Возврат оплаченного заказа. Возврат неоплаченного. Каждый сценарий -- отдельная ветка кода, отдельный набор потенциальных багов. Как и с транзакциями -- ошибки проявляются не в типовом сценарии, а в граничном. Подробнее об этом -- в разборе бага с ACID в 1С.
Один символ -- одна строка diff
Исправление заняло одну строку. Убрал инверсию из вызывающего кода:
// Было:
КоличествоВозврата = -СтрокаТовара.Количество;
// Стало:
КоличествоВозврата = СтрокаТовара.Количество;
Один символ. Минус. Удалён.
Можно было поступить наоборот -- убрать инверсию внутри функции. Но функция РаспределитьПоПартиям использовалась и в других местах, где вызывающий код передавал положительное количество, рассчитывая на инверсию внутри. Менять функцию -- ломать всех остальных вызывающих. Поэтому -- убрал лишний минус на стороне вызова.
Добавил комментарий к функции:
// Функция РаспределитьПоПартиям
// Количество: положительное число. Функция самостоятельно
// определяет направление движения (приход/расход)
// на основании контекста распределения.
Функция РаспределитьПоПартиям(Количество, ПараметрыРаспределения)
Теперь контракт явный. Следующий разработчик не будет гадать, передавать ли минус.
После исправления нужно было разобраться с накопленными неправильными данными. 34 документа возврата за месяц создали движения с неправильным знаком. Для каждого -- перепровести. Звучит просто, но перепроведение в рабочей базе с партионным учётом -- это цепочка: перепроведение возврата меняет остатки, изменение остатков может повлиять на FIFO-очередь последующих отгрузок, и так далее.
Сделал осторожно: выгрузил список затронутых документов, перепровёл в хронологическом порядке, после каждого -- проверил остатки по затронутой номенклатуре. Заняло полдня. Один минус -- полдня работы.
Вот что я вынес из этой истории.
Контракт функции должен быть явным. Какой знак ожидается на входе? Какой знак будет на выходе? Это не мелочь -- это критическая часть интерфейса. Комментарий в две строки экономит часы отладки.
Ответственность за знак -- в одном месте. Либо вызывающий код инвертирует, либо функция. Никогда -- оба. Это правило, которое нужно проверять на code review.
Редкие сценарии -- самые опасные. Баг в типовом сценарии находят за день. Баг в редком сценарии живёт месяцами. И чем дольше он живёт, тем больше данных портит, и тем дороже обходится исправление.
Математика -- не бухгалтерия. В математике минус на минус = плюс, и это нормально. В бизнес-логике возврат, который превращается в приход, -- это убытки, пересортица, ручная корректировка и нервный бухгалтер. Код не знает разницы между числом и его бизнес-смыслом. Эту разницу знает только разработчик.
Тридцать четыре документа. Один символ в коде. Месяц тишины. Если бы контракт функции был задокументирован -- баг не появился бы. Если бы частичный возврат тестировался -- баг нашли бы в первый день. Если бы кто-нибудь прогнал цепочку со знаком минус на бумажке -- увидел бы двойную негацию за минуту.
Но никто не проверил. Потому что код выглядел правильно. Минус перед количеством возврата -- это же логично, правда?
Правда. Логично. И внутри функции -- тоже логично. По отдельности -- каждый прав. Вместе -- баг.


