четверг, 1 октября 2015 г.

Работа над ошибками. Задача 1.10.

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


1) Модуль документа "Приходная накладная".
       Движения.ОстаткиНоменклатуры.Записывать = Истина;
Движения.Себестоимость.Записывать       = Истина;

Для Каждого ТекСтрокаСписокНоменклатуры Из СписокНоменклатуры Цикл
Движение = Движения.ОстаткиНоменклатуры.Добавить();
Движение.ВидДвижения = ВидДвиженияНакопления.Приход;
Движение.Период = Дата;
Движение.Номенклатура = ТекСтрокаСписокНоменклатуры.Номенклатура;
Движение.ЕдиницаИзмерения = ТекСтрокаСписокНоменклатуры.ЕдиницаИзмерения;
Движение.Склад = Склад;
Движение.Количество = ТекСтрокаСписокНоменклатуры.Количество;
КонецЦикла;

Для Каждого ТекСтрокаСписокНоменклатуры Из СписокНоменклатуры Цикл
Движение = Движения.Себестоимость.Добавить();
Движение.ВидДвижения = ВидДвиженияНакопления.Приход;
Движение.Период = Дата;
Движение.Номенклатура = ТекСтрокаСписокНоменклатуры.Номенклатура;
Движение.ЕдиницаИзмерения = ТекСтрокаСписокНоменклатуры.ЕдиницаИзмерения;
Движение.Количество = ТекСтрокаСписокНоменклатуры.Количество;
Движение.Сумма =ТекСтрокаСписокНоменклатуры.Сумма;
КонецЦикла;

Зачем 2 цикла? Конечно же все нужно запихнуть в 1 цикл.

Переходим к "Расходной накладной".

2)   Для каждого Стр Из СписокНоменклатуры Цикл
Если Стр.Номенклатура.ВидТовара = Перечисления.ВидыНоменклатуры.Услуга Тогда
Продолжить;
КонецЕсли;

Это неявный запрос в цикле. При такой конструкции каждый раз СУБД строит запрос в которой соединяет таблицу "Номенклатуры" с таблицей "ВидНоменклатуры". При большом количестве строк замедление может быть заметным.
Ил ладно бы еще эта проверка была нужна, но в запросе и так указано условие 
РасходнаяНакладнаяСписокНоменклатуры.Номенклатура.ВидТовара <> &ВидТовара
Так зачем же еще в было цикле добавлять это?.
Кроме того в условиях лучше использовать проверку на равенство, вместо условий "Не", "Не В", "< >".

3)  Можно было бы использовать ЗаполнитьЗначенияСвойств вместо
Движение = Движения.ОстаткиНоменклатуры.ДобавитьРасход();
Движение.Период           = Дата;
Движение.Склад            = Склад;
Движение.Номенклатура            = Стр.Номенклатура;
Движение.ЕдиницаИзмерения = Стр.ЕдиницаИзмерения;
Движение.Количество       = Стр.Количество;
но это не окажет никакого заметного влияния на производительность.

4) В самом начале табличная часть документа помещается во временную таблицу. И это хорошо. Но временную таблицу нужно индексировать по всем полям, которые будут участвовать в дальнейшем соединениях таблиц, а до изменений было только по номенклатуре.

5) В первом запросе есть неоптимальность которая называется "Соединение с виртуальной таблицей". Временная таблица соединялась с виртуальной. А ВТ тут вообще и не нужна, так как мы ни одного поля из нее не извлекаем.
Оптимальнее будет так:
ВЫБРАТЬ
ЕСТЬNULL(ОстаткиНоменклатурыОстатки.КоличествоОстаток, 0) КАК ОстатокКол,
ОстаткиНоменклатурыОстатки.Номенклатура.Представление,
ОстаткиНоменклатурыОстатки.Номенклатура КАК Номенклатура,
ОстаткиНоменклатурыОстатки.Склад КАК Склад,
ОстаткиНоменклатурыОстатки.ЕдиницаИзмерения КАК ЕдиницаИзмерения
ИЗ
РегистрНакопления.ОстаткиНоменклатуры.Остатки(
&ТочкаИтогов,
(Склад, Номенклатура, ЕдиницаИзмерения) В
(ВЫБРАТЬ
ДокТЧ.Склад,
ДокТЧ.Номенклатура,
ДокТЧ.ЕдиницаИзмерения
ИЗ
ДокТЧ)) КАК ОстаткиНоменклатурыОстатки
ГДЕ
ОстаткиНоменклатурыОстатки.КоличествоОстаток < 0




6)        Блокировка = Новый БлокировкаДанных;
ЭлементБлокировки = Блокировка.Добавить("РегистрНакопления.ОстаткиНоменклатуры");
ЭлементБлокировки.Режим = РежимБлокировкиДанных.Исключительный;
ЭлементБлокировки.ИсточникДанных = СписокНоменклатуры ;
ЭлементБлокировки.ИспользоватьИзИсточникаДанных("Номенклатура", "Номенклатура");
Блокировка.Заблокировать(); 

Я вообще-то не пью и не курю,но глядя на это верится с трудом) 
Блокировка не того регистра. Нужно блокировать регистр "СебестоимостьНоменклатуры".

7) Во втором запросе та же проблема, что и в первом "Соединение с виртуальной таблицей".
В отличие от первого запроса,в этом пригодятся поля из ВТ. Поэтому лучше поступить так.
Первым запросом пакета данные из виртуальной таблицы помещаем во ВТ и затем во втором запросе соединим полученные ВТ.
Запрос примет следующий вид:
ВЫБРАТЬ
ДокТЧ.Номенклатура,
ДокТЧ.ЕдиницаИзмерения,
ДокТЧ.Количество,
ДокТЧ.Склад,
ЕСТЬNULL(СебестоимостьОстатки.КоличествоОстаток, 0) КАК ОстатокКол,
СебестоимостьОстатки.Номенклатура.Представление,
ЕСТЬNULL(СебестоимостьОстатки.СуммаОстаток, 0) КАК ОстатокСум
ИЗ
ДокТЧ КАК ДокТЧ
ЛЕВОЕ СОЕДИНЕНИЕ РегистрНакопления.Себестоимость.Остатки(
&ТочкаИтогов,
Номенклатура В
(ВЫБРАТЬ РАЗЛИЧНЫЕ
ДокТЧ.Номенклатура
ИЗ
ДокТЧ)) КАК СебестоимостьОстатки
ПО ДокТЧ.Номенклатура = СебестоимостьОстатки.Номенклатура
И ДокТЧ.ЕдиницаИзмерения = СебестоимостьОстатки.ЕдиницаИзмерения




8)  Не решена "Проблема копеек".

               Если   Выборка.ОстатокКол <> 0 Тогда
Движение.Сумма = (Выборка.ОстатокСум/ Выборка.ОстатокКол) *Выборка.Количество;
КонецЕсли;

нужно как-то так

               Если Выборка.ОстатокКол =Выборка.Количество Тогда
Движение.Сумма = Выборка.ОстатокСум;
ИначеЕсли    Выборка.ОстатокКол <> 0 Тогда
Движение.Сумма = (Выборка.ОстатокСум/ Выборка.ОстатокКол) *Выборка.Количество;
КонецЕсли;
9)  Есть еще ошибка.

//Запрос.Параметры.Вставить("ТочкаИтогов", МоментВремени());
Дело тут в чем.  Когда мы очищаем движения методом "очистить" старые движения еще "висят" в памяти. И если мы оперативно перепроведем документ то получим ошибку, так как на остатках товар не появился. 
Следует добавить очистку регистра при оперативном проведении

Если РежимПроведения = РежимПроведенияДокумента.Оперативный Тогда
Движения.Себестоимость.Записать();
КонецЕсли;

10)  Если РежимПроведения = РежимПроведенияДокумента.Оперативный Тогда
Движения.Себестоимость.БлокироватьДляИзменения = Истина;
КонецЕсли; 
Эти строчки кода демонстрируют полное непонимание назначение свойства "БлокироватьДляИзменения", которое никак не связано с режимом проведения.

Свойство "БлокироватьДляИзменения" позволяет параллельно записывать одинаковые наборы данных в регистр, но использовать его нет большого смысла если после записи идет проверка остатков. Кроме того использование этого свойства может привести к взаимоблокировке по причине повышения уровня изоляции. Таким образом Для регистра "ОстаткиНоменклатуры" можно вообще отключить это свойство в регистре.(Так как после записи идет проверка). А для регистра "Себестоимость" такой проверки нет и это свойство имеет смысл установить. 

11) В отчете соединения виртуальных таблиц, что тоже не очень хорошо, лучше бы как и в документе помещать данные виртуальных таблиц во временные таблицы, которые между собой потом соединить.(Но этого я уже делать не буду).

Возможно остались еще какие-то моменты, но пока достаточно как мне кажется.


1 комментарий: