Задача на ошибки и неоптимальности при проведении приходной накладной

Задача:

Документ ПоступлениеТоваров при проведении делает движения по 2 регистрам:

• приход по регистру накопления ТоварыНаСкладах
• расход по регистру накопления ЗаказыПоставщикам

Запись в регистр накопления ТоварыНаСкладах делается в рублях (Сумма) и в долларах (СуммаВал). Курс берется на дату партии.

Документ может содержать в табличной части Товары тысячи строк с различными партиями.

Ниже представлена обработка проведения документа ПоступлениеТоваров.

Необходимо указать на ошибки и неоптимальные решения в процедуре проведения.

ВНИМАНИЕ, ТУТ ЗАЛОЖЕНО БОЛЕЕ 20 ОШИБОК И НЕОПТИМАЛЬНЫХ РЕШЕНИЙ, необходимо найти минимум 15.

Процедура ОбработкаПроведения(Отказ, РежимПроведения)
	Запрос = Новый Запрос;
	Запрос.Текст = "ВЫБРАТЬ
	| Товары.Номенклатура КАК Номенклатура,
	| Товары.Партия КАК Партия,
	| Товары.Количество КАК Количество,
	| Товары.Сумма КАК Сумма,
	| Заказы.КоличествоОстаток КАК КоличествоОстаток
	|ИЗ
	| Документ.ПоступлениеТоваров.Товары КАК Товары
	|ЛЕВОЕ СОЕДИНЕНИЕ
	|РегистрНакопления.ЗаказыПоставщикам.Остатки(&Период, Контрагент = &Контрагент) КАК Заказы
	| ПО Товары.Номенклатура = Заказы.Номенклатура
	| И Товары.Партия = Заказы.Партия
	|ГДЕ
	| Товары.Ссылка = &Ссылка
	| И (Заказы.Номенклатура, Заказы.Партия) В
	| (ВЫБРАТЬ
	| Товары.Номенклатура,
	| Товары.Партия
	| ИЗ
	| Документ. ПоступлениеТоваров.Товары КАК Товары
	| ГДЕ
	| Товары.Ссылка = &Ссылка)
	|ИТОГИ
	| СУММА(Количество),
	| СУММА(Сумма),
	| СУММА(КоличествоОстаток)
	|ПО
	| Номенклатура,
	| Партия";
	
	ВыборкаНоменклатура = Запрос.Выполнить().Выбрать();
	Пока ВыборкаНоменклатура.Следующий() Цикл
		Если ВыборкаНоменклатура.Количество > ВыборкаНоменклатура.КоличествоОстаток Тогда
			Сообщить("Количество в поступлении " + ВыборкаНоменклатура.Количество + " " + ВыборкаНоменклатура.Номенклатура.ЕдиницаИзмерения + " превышает остаток по заказам " 
			+ ВыборкаНоменклатура.КоличествоОстаток + " " + ВыборкаНоменклатура.Номенклатура.ЕдиницаИзмерения);
		Иначе
			ВыборкаПартия = ВыборкаНоменклатура.Выбрать();
			Пока ВыборкаПартия.Следующий() Цикл
				СуммаВал = СуммаВал	+ РегистрыСведений.КурсыВалют.ПолучитьПоследнее(ВыборкаПартия.Партия.Дата, Новый Структура("Валюта", Справочники.Валюты.НайтиПоНаименованию(«USD»)));
				Движение = РегистрыНакопления.ЗаказыПоставщикам.Добавить();
				Движение.ВидДвижения = ВидДвиженияНакопления.Расход;
				Движение.Период = Дата;
				Движение.Номенклатура = ВыборкаПартия.Номенклатура;
				Движение.Количество = ВыборкаПартия.Количество;
			КонецЦикла;
			Движение = Движения.ТоварыНаСкладах.Добавить();
			Движение.Период = Дата;
			Движение.Номенклатура = ВыборкаНоменклатура.Номенклатура;
			Движение.Количество = ВыборкаНоменклатура.Количество;
			Движение.Сумма = ВыборкаНоменклатура.Сумма;
			Движение.СуммаВал = СуммаВал;
		КонецЕсли;
	КонецЦикла;
КонецПроцедуры

Ответы:

1. Нет удаления старых движений документа.

2. Старые движения удаляем, если дата дока сдвинута вперед.

3. Нет установки флага для записи движений в конце транзакции.

4. Нет наложения управляемой блокировки.

5. Для минимизации времени блокировки запрос при помощи МенеджерВременныхТаблиц необходимо разбить на 2: а) получение ланных документа б) Получение остатков.

6. Нет установки параметров запроса.

7. В параметре на &период вирт. Таблицы «Остатки» следует использовать МоментВремени.

8. В случае оперативного поведения параметр «МоментВремени» следует установить в Неопределено.

9. Условие при выборке остатков на (Номенклатуру, Партию) следует делать в параметрах виртуальной таблицы.

10.  «РегистрыСведений.КурсыВалют.ПолучитьПоследнее(» - запрос в цикле. Курс нужно получать в запросе по таблице документа через макс период партии.

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

12.  Итоги не нужны.

13.  Т.к. остатка в регистре может не быть, поле «Заказы.КоличествоОстаток» следует обернуть в ЕстьNUll

14.  При выборке запроса не указан параметр «ОбходРезультатаЗапроса.ПоГруппировкам».(хотя идет именно по группировкам).

15.  Обход по группировкам не нужен.

16.  В случае превышения "Количество в поступлении" над остатком заказов, списания заказов по данной номенклатуре не происходит, что не логично: либо списываем что есть, либо не проводим документ.

17.  «ВыборкаПартия.Партия.Дата» - чтение всего объекта «через точку». При необходимости работы с полем дата его нужно добавить в запрос.

18.  «ВыборкаНоменклатура.Номенклатура.ЕдиницаИзмерения» - аналогично.

19.  «СуммаВал = СуммаВал + РегистрыСведений.КурсыВалют.ПолучитьПоследнее(» - странный алгоритм. Для вычисления валютной суммы вероятно стоит умножать Сумму в рублях на курс.

20.  +19 переменная СуммаВал не задана.

21.  «ЗаказыПоставщикам.Добавить();» - лучше использовать «ДобавитьРасход()»

22.  Не указан ВидДвижения при добавлении дв-ний ТоварыНаСкладах.

23.  При списании «ЗаказыПоставщикам» не задается партия.

Ответы от компании Софтвектор (извиняюсь за несоответствие номеров строк кода):

1.   Ошибка. Нет установки параметров для запроса. 3 шт.
2.   Строка 12-15. СОЕДИНЕНИЕ РегистрНакопления.ЗаказыПоставщикам.
    a. Нельзя соединяться с виртуальными таблицами в запросе. Следовало бы использовать сначала чтение во временную таблицу.
3.   Строка 9. Заказы.КоличествоОстаток КАК КоличествоОстаток при левом соединении здесь надо использовать ЕСТЬNULL(, 0). Иначе потом при «ВыборкаНоменклатура.Количество > ВыборкаНоменклатура.КоличествоОстаток» будет исключение о приведении типов
4.   Строка 34. ВыборкаНоменклатура = Запрос.Выполнить().Выбрать(); Здесь не совсем не точность, скорее просто небольшая оптимизация.  Когда каждая мс выполнения дорога, лучше сначала проверять что РезультатЗапроса не пустой, а потом уже тащить с сервера выборку. Иначе получается, что тащится выборка с сервера, а там пусто.
5.   Строка 18. И (Заказы.Номенклатура, Заказы.Партия) . Это параметры вирт таблицы, так что нужно их и вынести в параметры, чем писать их в области ГДЕ. + Там еще и лишние записи будут до соедеинения, т.к.
6.   Строка 23. Документ. ПоступлениеТоваров.Товары – Тут еще раз делается запрос к таблице. Лучше ТЧ товары сначала сохранить в ВТ и потом к ней обращаться.
7.   Строка 37. Сообщить – это скорее устаревший метод, он не работает на УФ, лучше использовать ОбщегоНазначения.СообщитьПользователю. Результат и на ОФ и На УФ будет одинаковым.
8.   Строка 39, 43. ВыборкаНоменклатура.Номенклатура.ЕдиницаИзмерения обращение через точку вызовет запрос к СУБД. Там конечно есть, какое то кэширование серверных значений. Но раз уж тут получаются данные через запрос и там будет номенклатура, то лучше сразу перенести в запрос.
9.   В качестве не оптимальных решений можно так же указать отсутствие установленных БлокировокДанных. Но тут вопрос скорее конфигурации. Может в ней стоит автоматический режим. Может этот документ создается один раз в год и ночью когда никто не работает(хотя это поступление, так что врятли, так что из разряда фантастики)
10.  Строка 49-50.
    a. СуммаВал = СуммаВал         +РегистрыСведений.КурсыВалют.ПолучитьПоследнее()
    b. Это тот же запрос, да еще к вирт. таблице, да еще и вызывается в цикле.  Надо переносить в основной запрос. Хотя бы как пакет, а еще лучше рассчитывать суммуВал для каждой строки в запросе.
11.  Строка 52. Справочники.Валюты.НайтиПоНаименованию(«USD»)  - это в цикле вызывается, и всегда возвращает одно и тоже значение. Лучше перенести поиск до цикла и потом просто его передавать
12.  Строка 52. Справочники.Валюты.НайтиПоНаименованию(«USD»)  - так же тут идет поиск по наименованию, что не верно. Лучше искать по коду. Или по гуиду. Или вообще завести как предопределенное значение, если его нет в конфе.
13.  Строка 52 - Новый Структура("Валюта", …)  тут еще момент, если не учитывать п.9 и п.10. То такую структуру лучше объявить до цикла и потом передавать её в функцию и если надо менять параметр. Тут параметр всегда один, так что лучше и все структуру один раз объявить и заполнить
14.  Строка 54, 61. Движение = РегистрыНакопления.ЗаказыПоставщикам  - перед записью движений надо очищать движения. Т.к. в случае перепроведения коллекция не очистится. Так же, сама коллекция движений явно не записывается, и не установлен флаг Записывать.
15.  Строка 49 – СуммаВал  = Сумма Вал  - Во первых, не инициализируется переменная. Во вторых – изза того что она не инициализацируется в выборке по номенклатуре её значение не «сбрасывается». К тому же, если цикл  ВыборкаПартия.Следующий() будет пустой, то наверное будет ошибка(вот точно не помню, по идее там неопределено будет, а приведется ли он к 0, не пробовал).
16.  Строка 52 - ВыборкаПартия.Партия.Дата – это тоже не явный запрос. Как в п.7
17.  Строка 61 Движения.ТоварыНаСкладах. – не установлен вид движения. Это явно рег. Остатков, так что там надо явно его установить. При проведении должно ругнуться
18.  Строка 39 ВыборкаНоменклатура.Количество – по одной номенклатуре может быть несколько партий, и ВыборкаНоменклаутра будет иметь итог больше чем запрошенное в документе количество. То же самое и с Суммой при формировании движений. И таже проблема с проверкой остатков ВыборкаНоменклатура.Количество > ВыборкаНоменклатура.КоличествоОстаток
19.  Ну еще момент в запросе, что нет группировки по номенклатуре и партии. Если в документе будет указаны одинаковые номенклатуры и партии в 2х строках, то будет не корректное списание партий, т.к. там же левое соединение по этим полям.