По следам код-ревью

17.09.19

Разработка - Рефакторинг и качество кода

+120 –

Приведу примеры с картинками и небольшим пояснением по вопросам, связанным с код-ревью (обзором кода).

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

Думаю для некоторой части аудитории предлагаемый список не привнесет особых открытий, поэтому буду рад если кто-то в комментариях предложит еще какие-нибудь «замечательные» случаи и рекомендации.

Ранее (очень давно) я уже писал обзор про часто встречающиеся ситуации (Типичные ошибки, некоторые вопросы качества и эффективности работы при разработке в 1С).

1. Используйте в команде код-ревью (обзор кода).

Если еще вы не ввели практику использования код-ревью в своей команде, то обязательно реализуйте. Нам без этого было реально сложно.

Если Вы "созрели" для проведения код ревью, то дерзайте. Некоторые аргументы для нашей команды:

1. Это мощная проверка того что делает джун или новичок.
2. Позволяет отсечь ошибки
Хочу заметить, тут нет цели найти все 100% ошибок (это вопрос тестирования), нашли несколько уже здорово.
3. Человек, который знает что его будут проверять, уже пишет лучше.
У нас есть инструкция для код-ревьювера на что он обязательно должен обращать внимание и если эти пункты нарушаются, то задача идет на возврат.
4. Все мы повышаем свою квалификацию.
После многократного разбора ошибок и методик "казусов" стало значительно меньше.
5. Уменьшается эффект bus factor (эффект автобуса)
6. Позволяет выбрать наиболее лучшее решение.
Проверяющий (щие) может не согласиться с реализацией и тогда проблема эскалируется и формируется коллегиальное обсуждение.

2. Используем «Связи параметров выбора» и «Параметры выбора»

для свойств реквизитов и свойств полей формы. Это в большинстве случаев делается легко и быстро особенно при доработках типовых конфигураций.

Если не делать никакого отбора, то получим вывод всех значений и возможность выбора даже не допустимых комбинаций. В качестве примера рассмотрим ограничение выбора для ставки НДС из типовой конфигурации.

И далее в модуле выбора мы видим код по обработке данной ситуации:

Такие же примеры можно привести для выбора номенклатуры, серии и т.д. И что самое замечательное большая часть необходимого кода уже существует)

Можно поставить проверку потом при записи формы или объекта, но лучшая практика не дать возможность выбрать лишних данных.

Замечу, что подобный подход не защищает от ошибки при программном создании.

2. Следуем общему стилю кода.

Очень важно следовать общему стилю кода – это делает доработки более прозрачными и качество высоким.

2.1. Форматирование. Это легко – выделите необходимый текст и нажмите «Shift+Alt+F».

2.2. Объявления названий реквизитов, общих модулей, объектов:

В типовых конфигурациях принято использовать параметр определяющий уникальность строки документа как «Код Строки». Но вот такая доработка "слегка" рушит общий подход.

С одной стороны, с точки зрения разработчика привнесена "большая" ясность в смысл названия, однако это значительно усложняет дальнейшую работу. Теперь покажем на примере, как это может быть.

Представим, что у нас есть новый документ с таблицей «Товары» и реквизитом «Код Строки Документа» в ней и к нам на вход приходит запрос (для примера переименовывать колонку не будем, можем забыть или результат типового запроса, который не меняем). И далее мы хотим заполнить наш документ.

Не забываем, что 1С рекомендует использовать процедуру «ЗаполнитьЗначенияСвойств» - это самый оптимальный вариант заполнения данных. И вместо вот этого кода:

Пока Выборка.Следующий() Цикл
  ЗаполнитьЗначенияСвойств(Объект.Товары.Добавить(),Выборка) 
КонецЦикла;

Нам придется делать так:

Пока Выборка.Следующий() Цикл
  стр_н = Объект.Товары.Добавить(); 
  ЗаполнитьЗначенияСвойств(стр_н,Выборка)
  стр_н.КодСтрокиДокумента = Выборка.КодСтроки; 
КонецЦикла;

2.3. Использование типовых подходов. Сформировать новые движения в регистр документа можно так (через конструктора):

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

Но лучше использовать типовой подход "Проведения документов" при выполнении процедуры проведения:

Процедура ОбработкаПроведения(Отказ, РежимПроведения)
...
    Документы.РеализацияТоваровУслуг.ИнициализироватьДанныеДокумента(Ссылка, ДополнительныеСвойства); // тут мы инициализируем таблицы для регистров
...
    ЗаказыСервер.ОтразитьЗаказыКлиентов(ДополнительныеСвойства, Движения, Отказ); // так выполняем запись
...
    ПроведениеСерверУТ.ВыполнитьКонтрольРезультатовПроведения(ЭтотОбъект, Отказ); // тут выполняется контроль
...
КонецПроцедуры

3. Используем «запросную модель», а не «объектную модель».

К примеру, у вас стоит задача проверить наличие признака «Налогообложение НДС» заказа клиента, и так делать не хорошо:

Если ЗаказКлиента.НалогообложениеНДС=ПредопределенноеЗначение("Перечисление.ТипыНалогообложенияНДС.ПродажаНаЭкспорт") Тогда
    // делаем что-то
КонецЕсли;

Первая проблема, с которой вы столкнетесь, то это снижение быстродействия, иногда существенное. А это все потому, что платформа 1С в данном случае получает весь объект целиком.
А если в документе существует реквизит с типом «Хранилище значения» и в нем хранится скан сертификата в 10 МБ?

Вторая возможная проблема, это ошибка по разграничению доступа. К примеру, кладовщик при оформлении ордеров может видеть некоторые реквизиты - ссылка, номер и др., но не должен обладать доступом к суммовым числам. Для этого Вы создали специальную роль без доступа к сумме документа и ценам - у вас тут гарантированно возникнет ошибка доступа по RLS.

Лучше делать так:

мНалогообложениеНДС = ОбщегоНазначения.ЗначениеРеквизитаОбъекта(ЗаказКлиента,”НалогообложениеНДС”); 

Если мНалогообложениеНДС=ПредопределенноеЗначение("Перечисление.ТипыНалогообложенияНДС.ПродажаНаЭкспорт") Тогда
       // делаем что-то
КонецЕсли;

Если проверять необходимо, даже при отсутствии прав, то не забываем использовать процедуру "УстановитьПривилегированныйРежим";

Объектная модель с остатками:

…
мОтбор = новый Структура();
мОтбор.Вставить("Склад",Склад);
// получим остатки
ТаблицаОстатков = РегистрыНакопления.ТоварыНаСкладах.Остатки(ТекущаяДата(),мОтбор,"Склад,Номенклатура","ВНаличии");
…

Так переписать код лучше:

Запрос = новый Запрос();
Запрос.Текст = "ВЫБРАТЬ
    |    Ост.Склад КАК Склад,
    |    Ост.Номенклатура КАК Номенклатура,
    |    Ост.ВНаличииОстаток КАК ВНаличииОстаток
    |ИЗ
    |    РегистрНакопления.ТоварыНаСкладах.Остатки(, Склад = &Склад) КАК Ост";
Запрос.УстановитьПараметр("Склад",Склад);
ТаблицаОстатков = Запрос.Выполнить().Выгрузить();

4. Используем возможности БСП.

В качестве примера рассмотрим программное задание отборов для динамических списков. (Не стоит изобретать велосипед, т.к. для большинства задач можно с успехом применять скрытые возможности БСП).

// Ищем группу
МассивГруппаИЛИ = ОбщегоНазначенияКлиентСервер.НайтиЭлементыИГруппыОтбора(Список.Отбор,,"ГруппаИЛИ");

// Добавляем если не найдена
Если МассивГруппаИЛИ.Количество()=0 Тогда

   ГруппаИЛИ = ОбщегоНазначенияКлиентСервер.СоздатьГруппуЭлементовОтбора(СписокПередачВозвратов.Отбор.Элементы, "ГруппаИЛИ", ТипГруппыЭлементовОтбораКомпоновкиДанных.ГруппаИли);
   ГруппаИ1 = ОбщегоНазначенияКлиентСервер.СоздатьГруппуЭлементовОтбора(ГруппаИЛИ, "ГруппаИ1", ТипГруппыЭлементовОтбораКомпоновкиДанных.ГруппаИ);
   ГруппаИ2 = ОбщегоНазначенияКлиентСервер.СоздатьГруппуЭлементовОтбора(ГруппаИЛИ, "ГруппаИ2", ТипГруппыЭлементовОтбораКомпоновкиДанных.ГруппаИ);
   ОбщегоНазначенияКлиентСервер.УстановитьЭлементОтбора(ГруппаИ1,"Организация",ВладелецТовара,ВидСравненияКомпоновкиДанных.Равно);
   ОбщегоНазначенияКлиентСервер.УстановитьЭлементОтбора(ГруппаИ2," Склад", Склад,ВидСравненияКомпоновкиДанных.Равно);

КонецЕсли;

// добавим к корню отбор
ОбщегоНазначенияКлиентСервер.УстановитьЭлементОтбораДинамическогоСписка(СписокПередачВозвратов,"Склад",Склад,ВидСравненияКомпоновкиДанных.Равно);

5. Используйте свойство «Пользовательская видимость»

для элементов формы.

5.1. При отладке. К примеру, когда вы разрабатываете новую обработку, то Вам для отладки может понадобиться увидеть различные поля и какую-то другую информацию технического характера. Пользователю они не нужны, а иногда вредны. В этом случае можно использовать небольшой фокус с пользовательской видимостью.

К примеру, создаете реквизит для себя, снимаете пользовательскую видимость как на рис. ниже.

Потом в своем сеансе включаете и и используете, а пользователь по умолчанию его не видит и скорее всего не знает о нем вообще.

5.2. Для форм выбора и списков. Обычно при создании форм конструктором форм ссылка не передается на форму и при выборе или выводе вы будете ограничены тем списком, который изначально выведен.

Соответственно, если пользователь захочет добавить на форму свои нужные ему поля, то он не сможет этого сделать. А вот если на форму добавить ссылку и снять у нее пользовательскую видимость, то мы оставляем пользователю возможность изменить состав колонок для списков.

6. Вывод первого поля табличной части документа в список.

Если вам требуется вывести в списках какое-либо поле для дополнительной информации из табличной части, к примеру, первую номенклатуру в документе «Этап производства» или первого партнера в «Задание на перевозку».

Вот так делать не стоит:

Запрос = новый Запрос();
Запрос.Текст = "ВЫБРАТЬ
|	ЭтапП.Ссылка КАК Ссылка,
|	ЭтапП.Номер КАК Номер,
|	ЭтапП.Дата КАК Дата,
|	ЭтапП.Статус КАК Статус,
|	ЕСТЬNULL(ВыхИзд.Номенклатура, ЗНАЧЕНИЕ(Справочник.Номенклатура.ПустаяСсылка)) КАК Номенклатура
|ИЗ
|	Документ.ЭтапПроизводства2_2 КАК ЭтапП
|		ЛЕВОЕ СОЕДИНЕНИЕ Документ.ЭтапПроизводства2_2.ВыходныеИзделия КАК ВыхИзд
|		ПО (ВыхИзд.Ссылка = ЭтапП.Ссылка)
|			И (ВыхИзд.НомерСтроки = 1)";

Оптимальным вариантом будет добавить в шапку документа новый реквизит и заполнять его при записи. Благодаря такому подходу вы с высокой эффективностью решите вопросы вывода дополнительной информации без существенного снижения производительности.

Код получится для запроса следующим:

Запрос = новый Запрос();
Запрос.Текст = "ВЫБРАТЬ
|	ЭтапП.Ссылка КАК Ссылка,
|	ЭтапП.Номер КАК Номер,
|	ЭтапП.Дата КАК Дата,
|	ЭтапП.Статус КАК Статус,
|	ЭтапП.НоменклатураПервойСтроки КАК Номенклатура // лучше реквизит назвать 'мм_НоменклатураПервойСтроки' 
|ИЗ
|	Документ.ЭтапПроизводства2_2 КАК ЭтапП";

7. Выбор и вывод дополнительных данных от полей составного типа

довольно серьезно влияет на производительность системы, особенно когда используются RLS. Чем большее количество типов, тем будет хуже вести себя система.

При решении подобной задачи удобно создать промежуточный регистр сведений (пример из типовых конфигураций - «Реестр Документов»). В этих регистрах хранится дополнительная информация «Номер документа», «Склад», «Организация», «Статус» и др. нужные информационные поля и дополнительно ссылка на документ или справочник (для связи).
В результате вместо тяжелого запроса RLS для составного типа будет использоваться только один простой.

8. Выполнение сложных вычислений в динамических списках/запросах.

Если требуется для каждого объекта в списке вывести сложно/трудоемко (или вообще не возможно запросом) вычисляемую информацию, к примеру, процент оплаты по заказам или состояние оформления документов, то выполнять эти вычисления необходимо отдельно, а не в запросе. Результат этих вычислений удобно хранить в промежуточном регистре сведений.

Примеры из типовой конфигурации - «Состояния заказов клиентов», «Состояние Отгрузки», «Состояния ЭД» и т.д.

Первоначально добавляется регистр с необходимыми полями и ссылкой на справочник или документ. Для рассматриваемого примера "механизм состояний заказов клиентов" существует регистр сведений «Состояния заказов клиентов»:

Далее при записи или проведении объекта вы добавляете процедуру для выполнения сложных вычислений в модуль объекта/ можно даже в отдельное регламентное задание (вообще будет работать монопольно, хороший пример - "расчеты с клиентами по документам").

Для документа Заказ Клиента эта процедура выглядит следующим образом (не забываем добавить в другие документы - поступления денежных средств и т.д.):

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

В результате запрос динамического списка получился простым, а состав выводимой информации "сложный":

9. Магическое число «семь плюс-минус два»

или куча переменных в параметрах функции. Пусть у вас есть процедура "СделатьЧтоТо", и Вы ее создаете заново или дорабатываете и получаете такую замечательную запись в итоге:

Процедура СделатьЧтоТо(Организация, Партнер, Контрагент, Соглашение, ДоговорКонтрагента, СуммаДокумента, НалогообложениеНДС и т.д. )
   ...
КонецПроцедуры

Стоп! Это перебор, необходимо остановиться и сократить количество параметров функции. Вынесите их в одну переменную с типом "Структура":

Процедура СделатьЧтоТо(Параметры) // Параметры - это структура с передаваемыми реквизитами.
  Организация = Параметры.Организация;
  Партнер     = Параметры.Партнер;
  ...
КонецПроцедуры

10. Излишняя сложность

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

Пусть у нас есть некая функция "СделатьЧтото(Параметр булево)", которая выглядит следующим образом:

Процедура СделатьЧтото(Параметр булево)
    Если Параметр=Истина Тогда
       // … что-то 1
       // к примеру, свернем таблицу товаров
       Товары.Свернуть("Номенклатура,Характеристика","Количество,КолчиествоУпаковок"); 
    Иначе
       // … что-то 2
       // к примеру, скроем видимость элементов на форме
       Элементы.Группа.Видимость = Ложь;
    КонецЕсли;
КонецПроцедуры

Вызывается в коде она в разных местах и при анализе кода без погружения в эту функцию понять что происходит будет довольно сложно (код должен читаться как хороший роман):

&НаСервере
Процедура ПриСозданииНаСервере(Отказ, СтандартнаяОбработка)
   …
   СделатьЧтото(Ложь);
   …
КонецПроцедуры


&НаСервере
Процедура ПередЗакрытиемНаСервере()
   …
   СделатьЧтото(Истина);
   …
КонецПроцедуры

Лучше всего такой код переделать на отдельные функции:

&НаСервере
Процедура ПриСозданииНаСервере(Отказ, СтандартнаяОбработка)
   …
   СкрытьЛишниеЭлементыНаФорме();
   …
КонецПроцедуры


&НаСервере
Процедура ПередЗакрытиемНаСервере()
   …
   ОбработатьТаблицуТовары();
   …
КонецПроцедуры


#Область ДополнительныеПроцедуры

&НаСервере
Процедура СкрытьЛишниеЭлементыНаФорме()
   // к примеру, скроем видимость элементов на форме
   Элементы.Группа.Видимость = Ложь;
КонецПроцедуры

&НаСервере
Процедура ОбработатьТаблицуТовары()
   // к примеру, свернем таблицу товаров
   Товары.Свернуть("Номенклатура,Характеристика","Количество,КолчиествоУпаковок"); 
КонецПроцедуры

#КонецОбласти

11. Дублирование кода.

На мой взгляд, это один из ужасных грехов в разработке. Из-за этого возникает большой технический долг и как следствие куча ошибок, особенно при выполнении доработки. Легко можем забыть где-то поправить изменения в куче дублирующихся или полу дублирующихся функций. Объем исправлений довольно большой и соответственно временные затраты на исправление.

В качестве примера рассмотрим следующую задачу: Первая часть. Вам необходимо добавить на некоторую форму реквизит типа булево с наименованием "Скрыть видимость дополнительных данных". При изменении на значение "истина" будет скрываться видимость группы "Групповая детализация". Делаем реализацию форма:

И код обработчика "при изменении":

&НаКлиенте
Процедура СкрытьВидимостьДополнительныхДанныхПриИзменении(Элемент)
	Элементы.ГруппаДетализация.Видимость = НЕ СкрытьВидимостьДополнительныхДанных;
КонецПроцедуры

Вторая часть задачи: Заказчик попросил сделать реквизит "Скрыть видимость дополнительных данных" сохраняемым на форме.

Мы переключаем свойство "Автоматическое сохранение данных" в "Использовать" и теперь, при открытии формы нам нужно изменить видимость. И сразу хочется написать следующий код:

&НаКлиенте
Процедура ПриОткрытии(Отказ)
	Элементы.ГруппаДетализация.Видимость = НЕ СкрытьВидимостьДополнительныхДанных;
КонецПроцедуры

Стоп. Так делать нельзя! В этот момент вы должны выделить код первой функции и нажать правую кнопку мыши и выбрать команду «рефакторинг». И вынести в отдельную процедуру. (Лениво, но сделать надо)

В итоге должно получиться так:

&НаКлиенте
Процедура СкрытьВидимостьДополнительныхДанныхПриИзменении(Элемент)
	СкрытьВидимостьДополнительныхДанных(); 
КонецПроцедуры


&НаКлиенте
Процедура ПриОткрытии(Отказ)
	СкрытьВидимостьДополнительныхДанных();
КонецПроцедуры

&НаКлиенте
Процедура СкрытьВидимостьДополнительныхДанных()	
	Элементы.ГруппаДетализация.Видимость = НЕ СкрытьВидимостьДополнительныхДанных;
КонецПроцедуры

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

&НаКлиенте
Процедура СкрытьВидимостьДополнительныхДанныхПриИзменении(Элемент)
	СкрытьВидимостьДополнительныхДанных(); 
КонецПроцедуры

&НаКлиенте
Процедура ПриОткрытии(Отказ)
	СкрытьВидимостьДополнительныхДанных();
КонецПроцедуры

&НаКлиенте
Процедура СкрытьВидимостьДополнительныхДанных()	
	Элементы.ГруппаДетализация.Видимость       = НЕ СкрытьВидимостьДополнительныхДанных;
	Элементы.ГруппаСуперДетализация.Видимость  = НЕ СкрытьВидимостьДополнительныхДанных;
КонецПроцедуры

Необходимо выработать правило на уровне ощущений, если чувствуете, что код повторяется, то надо остановиться и вынести дублирование в отдельную функцию.

Update: В статье Как завести у себя в команде код-ревью. Отвечаем на вопросы - отвечаем на организационные, технические и процессные вопросы. Статья построена в формате вопрос-ответ.

код-ревью практика программирование стиль кода

+120 –

См. также

"Чистый код в 1С" или как прокачать свой код? Пошаговая инструкция, часть №1

Рефакторинг и качество кода Программист Стажер Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

В последнее время термин «чистый код» стал очень популярным. Появились даже курсы по данной тематике. Так что же это такое?

16.09.2024 13961 markbraer 64

Обратная связь, атомарность и масштабируемость

Рефакторинг и качество кода Программист Бесплатно (free)

В статье рассматривается отказ от использования процедур и унификация формата ответа функций. Способ описывается на примере развития абстрактной информационной системы, работающей с PDF файлами.

10.09.2024 916 acces969 4

Быстрый фронт в базе размером 8.8 терабайт – наши стандарты при разработке компонентов системы

Рефакторинг и качество кода Бесплатно (free)

Для быстродействия большой базы данных важно не только оптимизировать запросы, но и соблюдать стандарты при разработке подписок на события, обработок для массового изменения данных, в реализации обработчиков обновления, расширений, регламентных заданий и в архитектуре СКД-отчетов. Расскажем о нюансах разработки компонентов большой системы.

28.08.2024 1163 Chernazem 3

Использование принципов SOLID в разработке на 1С

Рефакторинг и качество кода Программист Бесплатно (free)

SOLID – принципы проектирования программных структур (модулей). Акроним S.O.L.I.D. образован из первой буквы пяти принципов. Эти принципы делают код более гибким, упрощают разработку. Принято считать, что принципы SOLID применимы только в объектно-ориентированном программировании. Но их можно успешно использовать и в 1С. Расскажем о том, как разобраться в принципах SOLID и начать применять их при работе в 1С.

22.08.2024 10199 alex_sayan 41

Паттерн "Стратегия". Пример реализации

Рефакторинг и качество кода Программист Стажер Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

Рассмотрим основные принципы шаблона проектирования "Стратегия" на простом примере.

25.06.2024 4198 MadRave 34

Пример проведения Code-review #1

Рефакторинг и качество кода Программист Платформа 1С v8.3 Абонемент ($m)

В статье расскажу и покажу процесс проведения Code-review на примере обработки с GitHub.

1 стартмани

04.06.2024 6275 mrXoxot 55

Результаты ревью кода 1500+ решений каталога Инфостарт: наиболее частые ошибки разработчиков в коде

Рефакторинг и качество кода Платформа 1С v8.3 Бесплатно (free)

Поделюсь своим опытом аудита кода авторских продуктов с Infostart.ru как одним из элементов применения DevOps-практик внутри Инфостарт. Будет настоящий код, боевые скриншоты, внутренние мемы от команды ИТ-лаборатории Инфостарт и прочее мясо – все, что любят разработчики.

10.04.2024 13355 artbear 85

108

Ниндзя-код

Рефакторинг и качество кода Программист Платформа 1С v8.3 Россия Бесплатно (free)

Предлагаю вашему вниманию советы мастеров древности. Программисты прошлого использовали их, чтобы заострить разум тех, кто после них будет поддерживать код. Гуру разработки при найме старательно ищут их применение в тестовых заданиях. Новички иногда используют их ещё лучше, чем матёрые ниндзя. Прочитайте их и решите, кто вы: ниндзя, новичок или, может быть, гуру? (Адаптация статьи "Ниндзя-код" из учебника JavaScript)

01.04.2024 4234 DrAku1a 15

Комментарии

Подписаться на ответы Инфостарт бот

Свернуть все

1. muskul 10.07.19 04:09 Сейчас в теме

мОтбор = новый Структура();
мОтбор.Вставить("Склад",Склад);
// получим остатки
ТаблицаОстатков = РегистрыНакопления.ТоварыНаСкладах.Остатки(ТекущаяДата(),мОтбор,"Склад,Номенклатура","ВНаличии");
…
Так переписать код лучше:

Запрос = новый Запрос();
Запрос.Текст = "ВЫБРАТЬ
| Ост.Склад КАК Склад,
| Ост.Номенклатура КАК Номенклатура,
| Ост.ВНаличииОстаток КАК ВНаличииОстаток
|ИЗ
| РегистрНакопления.ТоварыНаСкладах.Остатки(, Склад = &Склад) КАК Ост";
Запрос.УстановитьПараметр("Склад",Склад);
ТаблицаОстатков = Запрос.Выполнить().Выгрузить();

Показать

Только вот первый работает во много раз быстрей.

2. nvv1970 10.07.19 06:46 Сейчас в теме

(1) этого не может быть, потому что не может быть.
Подтвердите свои слова техжурналом, планами и текстами запросов.
Если только у вас не отключены текущие итоги и нет миллионов записей после текущей даты.

17. muskul 10.07.19 09:10 Сейчас в теме

(2)Увидел такое в средней по объеме базе УТ когда получали долг контрагента по объекту расчета, были удивлены не меньше вашего.

18. nvv1970 10.07.19 09:15 Сейчас в теме

(17) я не удивлен.
Я говорю про то, что вы чего-то не увидели и не понимаете причин того, что увидели.
Запросы СУБД сравните для начала.

21. muskul 10.07.19 09:20 Сейчас в теме

(18)Зачем причины когда есть факт? прямое обращение к регистру остатков сработало быстрей.

85. swenzik 11.07.19 15:27 Сейчас в теме

(21) быстрее из-за того что в первом варианте явно указана ТекущаяДата(), добавьте в запрос параметром текущую дату и сравните

88. muskul 12.07.19 02:44 Сейчас в теме

(85)по вашему что я указывал в запросе? Была задача получить долг по документу. что то дернуло посмотреть как будет работать через регистр, увидел что он работает быстрей.

96. tormozit 7229 14.07.19 09:55 Сейчас в теме

(21) На основе результата одного эксперимента опасно делать вывода о преимуществе использованного в нем подхода. Вполне возможно, что твое наблюдение корректно и ты проверил отсутствие других влияющих факторов. Но даже в этом случае, опираясь на такое наблюдение не стоит утверждать, что так будет всегда. Неплохо было бы действительно увидеть запросы к СУБД из техножурнала. Это дало бы на порядок больше понимания. Я бы на твоем месте сделал это хотя бы для повышения собственной квалификации.

99. muskul 15.07.19 03:01 Сейчас в теме

(96)Так в чем проблема взять и проверить. Насчет повышения квалификации полностью согласен. Есть клиент у которого все как у всех а на неболшой ут в клиент сервере поиск отрабатывает то за 2 секунды то за пол минуты думает.

3. json 3349 10.07.19 07:47 Сейчас в теме

Автор, а как получилось, что код-ревью пропустило такое название документа "ЭтапПроизводства2_2" ?
Или у вас это считается нормальной практикой называть метаданные с цифрами на конце?

4. ivanov660 4577 10.07.19 07:52 Сейчас в теме

(3)Довольно забавный вопрос)
Пример взят из типовой конфигурации ERP 2.4.6 - это типовой решение от разработчиков компании 1С.

5. CSiER 36 10.07.19 07:56 Сейчас в теме

(3)

ЭтапПроизводства2_2

2_2 в данном случае означает версию конфигурации, которой соответствует логика работы документа.

7. AlexKo 102 10.07.19 08:08 Сейчас в теме

(3)
Это объект типовой конфигурации ERP 2 1С.
Очень спорное решение архитектора который отвечает в 1С за подсистему производства.
Если в случае ЗаказНаПроизводство2_2 можно натянуто оправдать тем, что оставили объект ЗаказНаПроизводство в конфигурации, то "Этапов производства" не было ранее, были маршрутный листы. Мне не нравится такое решение в типовой.

9. ivanov660 4577 10.07.19 08:19 Сейчас в теме

(7)Чисто теоретически - вдруг архитектор из 1С вдруг ответит в этой ветке)

6. genayo 10.07.19 08:02 Сейчас в теме

С дублированием кода всё не так однозначно, как говорит нам код типовых конфигураций, где этого дублирования выше крыши :))

8. ivanov660 4577 10.07.19 08:17 Сейчас в теме

(6)
1. На мой взгляд однозначно - его надо искоренять. Плюсов его наличия я не вижу.
2.Они между прочем с этим борются (по крайней мере я так предполагаю), но так как объем БСП большой и "сложный" это не быстрый процесс.

11. genayo 10.07.19 08:34 Сейчас в теме

(8) Тут был один защитник дублирования - говорил, что единая процедура проведения для нескольких документов зло, вносить изменения на порядок сложнее. Я с ним не совсем согласен, конечно, но преднамеренное избегание дублирования кода должно быть учтено при проектировании архитектуры, мне кажется. Чего с БСП изначально не произошло.

14. ivanov660 4577 10.07.19 08:57 Сейчас в теме

(11)Для исправления дублирования кода существует практика рефакторинга, т.ч. у них еще есть шанс) сделать код лучше.
Мне нравится сайт с классным описанием методик по рефакторингу refactoring.guru

15. genayo 10.07.19 09:01 Сейчас в теме

(14) Рефакторить архитектуру продукта может быть весьма накладно...

97. tormozit 7229 14.07.19 10:11 Сейчас в теме

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

98. ivanov660 4577 14.07.19 10:43 Сейчас в теме

(97)Из-за ограничений 1С и принципа доработки типовых конфигураций, конечно удобнее дублировать код в разумных рамках (чтобы не менять типовой и не париться при изменениях БСП).
Однако, в рамках своих доработок и разработок искоренять дублирование необходимо, а для вопроса согласованности требуется использовать договоренность в рамках некоторого интерфейса (паттерн фасад). Т.е. все эти сложные блоки функционала должны иметь удобный интерфейс - хороший пример far и набор плагинов.
А так получится индусский код, когда каждая команда пишет для себя все что нужно не глядя на то накодили другие. С таким ужасом я впервые (очень давно) столкнулся еще при доработках Битрикса.

10. Lucifer93 95 10.07.19 08:22 Сейчас в теме

А что, простите, сложного? Можно увидеть пару аргументов за код-ревью? У нас просто код-ревью не проводится хотелось бы услышать чем становится легче.

12. genayo 10.07.19 08:35 Сейчас в теме

(10) Аргументы для кого? Для разработчика, заказчика продукта, проджект-менеджера?

13. Lucifer93 95 10.07.19 08:38 Сейчас в теме

(12) Хотелось бы услышать комплекс аргументов, просто я, как разработчик, не очень понимаю зачем делать код-ревью. Мне всегда казалось, что достаточно придерживаться стандартов разработки принятых в компании. Хотелось бы понять что даст код-ревью.

16. genayo 10.07.19 09:04 Сейчас в теме

(13) Вот, например https://habr.com/ru/post/250489/

19. Lucifer93 95 10.07.19 09:17 Сейчас в теме

(16) Спасибо, прочитал. Жаль, что автор статьи не указал никаких аргументов в этом пункте, так как не всем необходимо код-ревью.

112. Светлый ум 415 20.09.22 05:44 Сейчас в теме

(16) сохранил для себя

Ревью кода помогает:
- Находить баги
- Обеспечивать читаемость и поддерживаемость кода
- Распространять знания кода на всю команду
- Быстрее входить в работу новым членам команды
- Показывать всем новые подходы к решению задач

26. ivanov660 4577 10.07.19 09:34 Сейчас в теме

(13)
(19)
В принципе данное утверждение не должно нуждаться аргументации, т.е. я вас не заставляю, если "созрели", тогда дерзайте. Вот что дало нам (несколько примеров):

1. Это мощная проверка того что делает джун или новичок.
2. Позволяет отсечь ошибки
3. Человек, который знает что его будут проверять, уже пишет лучше.
У нас есть инструкция для код-ревьювера на что он обязательно должен обращать внимание и если эти пункты нарушаются, то задача идет на возврат.
4. Все мы повышаем свою квалификацию.
После многократного разбора ошибок и методик "казусов" стало значительно меньше.
5. Уменьшается эффект bus factor (эффект автобуса)
6. Позволяет выбрать наиболее лучшее решение.
Проверяющий (щие) может не согласиться с реализацией и тогда проблема эскалируется и формируется коллегиальное обсуждение.

33. Lucifer93 95 10.07.19 09:44 Сейчас в теме

(26) Да, я понял, что плюсы есть. Просто в моем случае это не даст особого выхлопа с учетом специфики организации. Спасибо за аргументы, думаю, что их можно включить в первый пункт статьи, чтобы было удобнее.

В принципе данное утверждение не должно нуждаться аргументации, т.е. я вас не заставляю, если "созрели", тогда дерзайте.

Ваши аргументы как раз и дали мне понять, что мы еще не созрели.

36. ivanov660 4577 10.07.19 09:46 Сейчас в теме

(33)Спасибо, попозже поправлю статью с учетом всех замечаний)

63. s_vidyakin 68 11.07.19 10:56 Сейчас в теме

(13) а зачем вы придерживаетесь стандартов разработки? Хотелось бы услышать комплекс аргументов, мне всегда казалось что достаточно просто иметь внятное ТЗ и понимать задачу.

66. TODD22 19 11.07.19 11:02 Сейчас в теме

(63)

а зачем вы придерживаетесь стандартов разработки?

Если для вас не очевидно зачем придерживаться стандартов то никакой "комплекс аргументов" вас не убедит.

ведь достаточно же просто иметь внятное ТЗ и понимать задачу.

ТЗ и понимание задачи это то что надо сделать, стандарты это как надо сделать или как не надо делать и как оформить то что сделал.

77. s_vidyakin 68 11.07.19 11:55 Сейчас в теме

(66) тогда код-ревью это проверка, что кто-то сделал так, как надо сделать, не сделал то, что не надо делать и оформил то что сделал, логично?

78. TODD22 19 11.07.19 12:04 Сейчас в теме

(77)Логично, но это только часть того что даёт код ревью команде.
Это ещё и подтягивание всей команды до одного уровня, общее владение кодом,, изучение лучших и худших практик в одной команде.

80. starik-2005 3087 11.07.19 12:37 Сейчас в теме

(77) вообще, логично любой проект защищать. Код-ревью - это защита проекта перед некой приемной комиссией в лице ведущего разработчика или архитектора. Выдвижение формальных требований к оформлению кода, конечно, достаточно бессмысленная процедура (например, именование переменных, функций, псевдонимов в запросах, оформление комментариев или иные моменты, которые не влияют на функционал), а вот функциональные требования к коду, его достаточность в части оптимальности, его влияние на общую производительность решения - это должно быть оценено и, в случае необходимости, подвергнуто сомнению и защищено автором от подобных сомнений путем аргументации. Сам по себе процесс ревью уже даст команде развитие, как один из драйверов прокачки и "софт скиллов", и навыков разработки.

81. TODD22 19 11.07.19 12:44 Сейчас в теме

(80)

например, именование переменных, функций, псевдонимов в запросах, оформление комментариев или иные моменты, которые не влияют на функционал

Код довольно часто читают. Кто то пишет как положено по стандарту "ПолучитьКурсВалют", а кто то пишет "ПолучаемКурсыВалют", третий пишет "ПолучаюКурсыВалют", четвёртый "ЗапросКурсовВалют".

Как то в коде одного коллеги "Если Не МыНеНаСервере Тогда".

106. user774630 17.09.19 10:41 Сейчас в теме

(81)
https://its.1c.ru/db/v8std#content:647:hdoc

4. Не рекомендуется в названиях процедур и функций описывать типы принимаемых параметров и (или) возвращаемых значений.

Например, неправильно:

Функция ПолучитьМассивРолейСПравомДобавления()
Функция ПолучитьСтруктуруДополнительныхНастроек()
Правильно:

Функция ИменаРолейСПравомДобавления()
Функция ДополнительныеНастройки()

20. zqzq 25 10.07.19 09:20 Сейчас в теме

Произвести уборку путём сгребания мусора под ковёр... По мне так лучше отдельные параметры оставить.

25. json 3349 10.07.19 09:30 Сейчас в теме

(20) очень удачная метафора в данном случае

28. ivanov660 4577 10.07.19 09:38 Сейчас в теме

(20)Не соглашусь, не зря пункт называется "магическое число семь плюс-минус два". Возможно в некоторых случаях требуется пересмотреть архитектурное решение - провести рефакторинг, но не всегда это возможно (при доработках типовых проблематично)

65. s_vidyakin 68 11.07.19 11:02 Сейчас в теме

(20) Нет, т.к. большое количество параметров это повод задуматься, а зачем так много входных данных для процедуры, скорее всего это сваленные в кучу несколько несвязанных операций и надо попробовать разделить их на логичные неделимые шаги с 1-3 параметрами

22. PLAstic 296 10.07.19 09:23 Сейчас в теме

Весь документ пестрит обращениями к значениям перечислений. Почему-то никто из команды код-ревью не прочитал справку, где написано:

ПредопределенноеЗначение(<ИмяПредопределенногоЗначения>)

Описание:

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

Примечание:

Результат выполнения кэшируется при первом обращении до изменения конфигурации или версии платформы.

Раз уж мы говорим про оптимизацию быстродействия...

29. ivanov660 4577 10.07.19 09:39 Сейчас в теме

(22)Да, это более правильный вариант записи условия сравнения. Но пример делает акцент на другой задаче.

23. OerlandHue 10.07.19 09:25 Сейчас в теме

Я не согласен с Вами по поводу пункта 2.1. Незачем мозги компосировать разработчику, один пробел или два, один перенос строки или два. В прочих языках это решается кастомной настройкой отображения в редакторе кода. В 1С последние года два очень модно говорить про общие правила форматирования кода, по итогу это решается так, что сначала один руководитель приходит ставит свои правила форматирования ты переучиваешься, потом другой и в итоге получается вообще полное дерьмо, ни разработчик не удовлетворен, ни единого кода нет.
Читаемость кода это НЕ улучшает, сами посмотрите, по-моему, если и есть какая-то разница в скорости чтения всех трех вариантов, то такая маленькая разница в скорости чтения доступна только НИИ.
Хватит эту тему активно педалировать в угоду своему самомнению.

Остальное в плюсик. Полезные практики.
Хотя, опять же, по поводу code review. Надо не к нему стремиться, а стремиться его автоматизировать. Нет ничего хорошего тратить кучу времени на ручной отлов технического долга и ошибок.

Прикрепленные файлы:

27. genayo 10.07.19 09:37 Сейчас в теме

(23) Скажите честно, вам нравится форматирование в типовых конфигурациях от 1С?

34. PLAstic 296 10.07.19 09:45 Сейчас в теме

(27) В основном да. Что не поощряю - междустрочное выравнивание присвоений.

Объект.ЭтоГрамотноНазванныйРеквизитОбъекта = ИсточникЗаполнения.НекийРеквизит;
Объект.Код                                 = ИсточникЗаполнения.Код;

Глазами теряется, что же мы присваиваем коду.

42. OerlandHue 10.07.19 10:52 Сейчас в теме

(34) а на мой взгляд это одинаково совершенно. Кстати, у нас тимлид запретил делать такое выравнивание. Вот вам и правила кода, подстраивайся под хотелки каждого.

43. ivanov660 4577 10.07.19 11:04 Сейчас в теме

(42)Почему подстраивайся под хотелки каждого, тим лид ввел на всю команду одно правило. Код более будет соответствовать внутри командному стандарту.
Другое дело если две и более команд с разными стандартами, то это жесть. Особенно если задачки будут мигрировать между ними.

49. OerlandHue 11.07.19 06:40 Сейчас в теме

(43) это будет не командный стандарт, а стандарт тим лида. Ему так удобно всю жизнь было и теперь все будут переучиваться писать как он. Потом тимлид сменится и все будут переучиваться за следующим тимлидом. При том, что разницы в чтении кода нет.
Приведу Вам пример из личного. Один руководитель писал всю жизнь запятые при переносе параметров перед параметром. Пришлось переучиваться под его стандарт.
Потом другого члена команды сделали ответственным за правила написания кода. И вуаля, у нас новый стандарт, теперь нужно ставить запятую на строке после параметра. И это кажется, что легко переучиваться, по факту приходится весь код обходить и переставлять запятые.
Круто наверное начитавшись о коннела и прочих РУКОВОДИТЕЛЕЙ программистов внедрять всякие "корпоративные стандарты".
Не круто говорить потом про "команду", как удобно стало "команде" и удивляться, почему мало кто принимает участие в разработке стандартов, почему со скепсисом разработчики относятся к таким вещам.
И этих примеров валом.
Оставьте эти вещи хаотичному порядку. Люди сами начинают писать примерно одинаково, когда работают вместе, без требования от тимлида ставить два пробела после запятой или прочей ерунды. Как это произойдет с пробелами после и до операторов сравнения. Люди сами попросят другого программиста так делать, без Вашего вмешательства и если это действительно мешает работе, то надавят (но на самом деле даже пробелы после и до операторов сравнения может и бесят, но на читабельность не влияют).

Ответить

51. starik-2005 3087 11.07.19 09:14 Сейчас в теме

(49) а Вы думали, почему запятые лучше размещать перед параметром? Поверьте, в этом есть гоубокий смысл. MS SQL Men.Studio делает так не от просто потому что)))

53. OerlandHue 11.07.19 09:26 Сейчас в теме

(51) Я не знаю про это. У нас есть требование, например, писать текст запроса всегда с отдельной строки, для корректного отображения изменений в git. Я согласен с такими требованиями, потому что это не вкусовщина, это помогает смотреть изменения по проекту, blame и filehistory.
А про запятые в переносе параметров это выглядит конкретно у нас как вкусовщина, потому что никому в команде неизвестен потаенный смысл, зачем так делать. Если Вы скажете, почему лучше делать именно, то я буду благодарен.
(52)

Даже если он неидеален - наличие стандарта лучше, чем его отсутствие.

Я придерживаюсь другого мнения, что чем меньше правил в стандарте, тем лучше. Иначе ваш стандарт никто не запомнит и пользоваться им не будет. А Вы узнаете об этом только через год, когда случайно заглянете в чужой код.

54. starik-2005 3087 11.07.19 09:31 Сейчас в теме

(53) ну тут все на поверхности и пытливому уму должно быть понятно, что пр удалении строки с параметром запятая от предыдущего параметра не останется висеть в конце предыдущей строки. Для запросов выбору умные люди уже давно так делают, а вот для функций и процедур - да, смысла немного...

55. OerlandHue 11.07.19 09:33 Сейчас в теме

(54)

ытливому уму должно быть понятно, что пр удалении строки с параметром запятая от предыдущего параметра не останется висеть в конце предыдущей строки. Для запросов выбору умные люди уже давно так делают, а вот для функций и процедур - да, смысла немного...

Действительно удобно :)
Для 1С наверное не подойдет, потому что есть привычка пользоваться конструктором запроса и он такое сотрет.

105. Alien_job 190 31.07.19 19:20 Сейчас в теме

(54) Это ухудшает читаемость, а хороший код принято больше читать чем переписывать

52. herfis 513 11.07.19 09:18 Сейчас в теме

(49)

Оставьте эти вещи хаотичному порядку. Люди сами начинают писать примерно одинаково, когда работают вместе, без требования от тимлида ставить два пробела после запятой или прочей ерунды.

Да-да, конечно же, так всегда и бывает (красненьким). Если тимлиду удается прогнуть остальную команду под свой стиль - это и будет командный стандарт. Даже если он неидеален - наличие стандарта лучше, чем его отсутствие. Трудности "переучивания" преувеличены. Нормальному разрабу не составляет никакого труда следовать навязанному стилю. А вот смешение стилей не просто бесит - оно реально отвлекает и снижает читабельность (и именно поэтому бесит).

57. TODD22 19 11.07.19 09:55 Сейчас в теме

(49)

Потом тимлид сменится и все будут переучиваться за следующим тимлидом.

Обычно наследуют практику "старого" тим лида или переделывают всё под стандарты нового, но это уже экстрим, а два стандарта в одном проекте это жесть.
И "грамотный" тим лид вряд ли возьмётся под своё виденье прекрасного переделывать большой проект. Это всё таки большой объём работы, затраты времени, риски, ответственность и тд.
Проще "новому" тим лиду принять стандарты старого.

Люди сами попросят другого программиста так делать, без Вашего вмешательства и если это действительно мешает работе, то надавят

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

Люди сами начинают писать примерно одинаково, когда работают вместе

С соглашением они начинают это делать гораздо качественнее и быстрее.

30. genayo 10.07.19 09:41 Сейчас в теме

(23) Кстати да, ошибки в именовании переменных, например, и в форматировании, вылавливать на коде ревью нерационально, это точно можно автоматизировать.

35. ivanov660 4577 10.07.19 09:45 Сейчас в теме

(30)Инструменты в студию

40. genayo 10.07.19 10:04 Сейчас в теме

(35) Сам не пользовался, конечно, но беглый гуглинг даёт https://github.com/znsoft/1SCodeAnalyze https://infostart.ru/public/527258/

44. ivanov660 4577 10.07.19 11:05 Сейчас в теме

(40)Хотелось бы уровня pvs-studio.

45. genayo 10.07.19 11:50 Сейчас в теме

(44) У серебряных пулемётчиков вроде есть что-то...

31. ivanov660 4577 10.07.19 09:42 Сейчас в теме

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

24. json 3349 10.07.19 09:27 Сейчас в теме

Автор, опиши еще организационные аспекты код-ревью.

У тебя в статья пока как-то слабо соответствует заголовку.
Сначала даешь определение, потом пишешь, что код-ревью проводить надо, аргументируя это лишь тем, что вам без него было сложно.
И все, дальше тема код-ревью перетекает в тему рефакторинга.
И вся остальная статья про рефакторинг.

Интереснее как такое внедрить в команде:
- кто проводит
- с какой периодичностью
- как исправляете (задачу создаете или еще как)
- как контролируете выполнение исправления (вдруг разработчик просто забил на замечание)
- как боретесь с вкусовщиной (когда у разных разработчиков разное понимание стандартов)
- всех проверяете или выборочно

Ответить

32. ivanov660 4577 10.07.19 09:44 Сейчас в теме

(24)По чему не соответствует - по следам код-ревью. Я вкладывал мысль вот мы проводили вебинары внутри команды, составили список и решили поделиться с сообществом теми вопросами, которые обсуждали.
А первый пункт это такой призыв)

37. PLAstic 296 10.07.19 09:47 Сейчас в теме

(32) Но осветить вопросы, поставленные оппонентом всё же стОит. Это для многих будет интересно.

39. json 3349 10.07.19 09:52 Сейчас в теме

(32) согласен, не внимательно прочитал.
Заголовок соответствует

Но про организационные аспекты было бы не менее интересно узнать.

На каких проектах код-ревью целесообразно применять?
Какие трудности возникают?
Как долго применяете?
Сколько времени тратится и каких специалистов?

Очень полезно было бы узнать ответы на эти вопросы (возможно в следующей публикации).

41. TODD22 19 10.07.19 10:05 Сейчас в теме

(39)

На каких проектах код-ревью целесообразно применять?

На любых где есть команда работающая над одним продуктом.

Код-ревью очень хорошо повышает уровень программистов, правда до уровня самого компетентного, но иногда есть "синергетический" эффект. Когда уровень всей команды начинает расти, если все активно включаются в процесс.

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

61. json 3349 11.07.19 10:50 Сейчас в теме

(41)
Если в команде нет джунов и мидлов. Все ведущие. Любой может сделать любую задачу и сделать хорошо.

Нужно ли выполнять код-ревью?

Каждый знает стандарты и соблюдает.
Но при проведении код-ревью возникают только вопросы с расхождением в толковании стандартов. Каждый считает правильным писать так, как он привык. И этому человеку бесполезно доказывать, что ты считаешь по другому (потому что ты так привык).

Какая будет польза от организации код-ревью в таких случаях?

62. TODD22 19 11.07.19 10:56 Сейчас в теме

(61)

Каждый знает стандарты и соблюдает.

Но при проведении код-ревью возникают только вопросы с расхождением в толковании стандартов. Каждый считает правильным писать так, как он привык.

Нужно ли выполнять код-ревью?

Если читать вот в таком порядке то ответ будет очевиден.

И этому человеку бесполезно доказывать, что ты считаешь по другому (потому что ты так привык).

Для этого и нужно общее соглашение и один человек отвечающий за это и требующий соблюдения правил от других иначе "лебедь, рак и щука". Что бы не надо было никому ничего доказывать. У нас впёртых совсем не было. Был один его очень быстро уволили как не прошедшего испытательный срок.

Любой может сделать любую задачу и сделать хорошо.

Сделать хорошо можно двумя способами, с соблюдением соглашений и без соблюдения. "Хорошо" это когда работает. "Соглашения" это требования к оформлению и написанию кода.

70. json 3349 11.07.19 11:14 Сейчас в теме

(62)
(67)
Мы возможно друг друга не понимаем.

Есть несколько человек. Когда открываешь их код, то видишь, что пишут они круто. И с читаемостью полный порядок и с оптимальностью. Может есть какие-то неудобства, но ты понимаешь, что это только потому, что этот человек привык по-другому чем ты (например, префиксы временным таблицам давать другие, выравнивать/не выравнивать текст запроса после равенства и другие подобные мелочи, по другому группировать процедуры по областям).

И какой же смысл ковыряться в таком коде и выискивать косяки ?
Ведь код-ревью, это другими словами, выискивание косяков.
А если косяки встречаются редко, то какой смысл их выискивать?

Дешевле исправить их во время обнаружения или я не прав?

74. TODD22 19 11.07.19 11:29 Сейчас в теме

(70)

Ведь код-ревью, это другими словами, выискивание косяков.

Не только, это ещё перенимание хороших подходов.
Тут дело не в том что все пишут круто, но по своему, тут надо что бы все писали круто и при этом одинаково.

Общее владение кодом надо повышать, если работаете над одним проектом. Что бы потом можно было без проблем передавать задачи от одного человека другому внутри команды. А не так что тут писал Петя вот он выйдет из отпуска пусть сам в своей писанине и разбирается.

Есть такая вещь как стандартизация рабочего места. Есть много магазинов в них работают замечательные продавцы, каждый из них "круто" продаёт. Но существует проблема, когда один продавец выходит в другой магазин. Он не знает где что лежит. По этому рабочие места магазинов унифицируются, документы он должен складывать в зелёную папочку которая должна лежать справа от кассы в ящике, служебные записки он должен класть в жёлтую папочку которая лежит там же. Инвентарь лежит в определённом месте, чековая лента то же в определённом и тд.

Не обязательно ревьюить весь код. Можно это делать например раз в неделю по какой то части кода, остальное отдавать на откуп статическим анализаторам кода.

Да и вообще код ревью способствует так сказать "сплачённости" коллектива. Превратите "код ревью" в небольшой пятничный "тим билдинг" и профит не заставит себя долго ждать.

92. PLAstic 296 12.07.19 11:05 Сейчас в теме

(74) А я же правильно понимаю, что КР даёт ещё достаточно глубокое погружение проверяющего в реализуемые механизмы проверяемого? Т.е. позже проверяющий более активно сможет использовать механизмы проверяемого и в своей работе.

Просто, у нас есть такая проблема, что один пишет полезную для других функцию или механизм, а другие про это не знают. При реализации подсистем, конечно, оповещение по почте идёт, но не будешь же рассылку делать при реализации каждой процедуры/функции?

93. TODD22 19 12.07.19 11:12 Сейчас в теме

(92)

Просто, у нас есть такая проблема, что один пишет полезную для других функцию или механизм, а другие про это не знают.

:)

Это уже надо как то организовывать что то типа "общей библиотеки" с документацией и обязательным ознакомлением.

75. TODD22 19 11.07.19 11:31 Сейчас в теме

(70)

Дешевле исправить их во время обнаружения или я не прав?

Это зависит от того дешевле ли их исправить во время обнаружения. Исправлять что то в базе которая работает 24*7 и у тебя в неделю одно окно на 3 часа для установки обновлений может быть очень не дёшево.

67. starik-2005 3087 11.07.19 11:03 Сейчас в теме

(61)

Какая будет польза от организации код-ревью в таких случаях?

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

Но вообще, просмотр кода - это и трансляция хороших решений в команду. Т.е. не только ошибки разработчиков выявляются, но и их фишки могут обогатить команду опытом.

Т.е. всегда есть два потока - восходящий и нисходящий, поток ошибок и интересных решений. Код-ревью - отличный транслятор опыта в команду.

69. AntonSm 30 11.07.19 11:12 Сейчас в теме

(67) вот тут небезызвестный Лустин говорит, что кодревью это дорого, очень дорого.
Т.е. вы считаете, что кодревью стоит затрат, потраченных на него, в любом случае?

72. starik-2005 3087 11.07.19 11:20 Сейчас в теме

(69) небезызвестный Макконнелл говорит, что код-ревью в 4 раза дешевле тестирования и в 2 раза эффективнее. Даже и не знаю, кому из этих авторитетов верить...

76. AntonSm 30 11.07.19 11:32 Сейчас в теме

(72)

Даже и не знаю, кому из этих авторитетов верить...

Вроде оба дядьки умные.
Но Лустин говорит про сегодняшние реалии.
Еще и в контексте применения автотестов всяких.
У Макконнелла про автотесты было что-то, не подскажите?
Непонятно, когда эти автотесты появились, до или после подсчета стоимости тестирования.

79. starik-2005 3087 11.07.19 12:29 Сейчас в теме

(76)

У Макконнелла про автотесты было что-то, не подскажите?
Непонятно, когда эти автотесты появились, до или после подсчета стоимости тестирования.

У Макконнелла про тестирование было то, что это дорого. На Боинге тестирование стоило в 8 раз дороже, чем ревью кода, но мы то знаем, где сейчас Боинг с их МАХ (правда, это только с ним такие проблемы, и я лучше на самолете производства Боинг полечу, чем на суперджете, где, предположу, толком нет ни того, ни другого).

Автотесты сейчас почти во всех проектах есть, но их наличие позволяет найти в большинстве своем те проблемы, которые уже были когда-то обнаружены, при том не единожды. И есть у Макконнелла мнение, основанное на исследованиях разработки ПО, что тестирование получается сильно дороже экстремальной разработки с использованием парного программирования и гибких подходов, ревью кода в которых считается неотьемлемой частью процесса постоянного улучшения качества.

94. qwinter 683 12.07.19 15:49 Сейчас в теме

(79)

На Боинге тестирование стоило в 8 раз дороже, чем ревью кода

Так на МАХ было именно КР, и минимум тестов. И модуль в котором ошибка более 5 раз переписывали подгоняя под стандарты разработки.

95. starik-2005 3087 12.07.19 17:49 Сейчас в теме

(94)

И модуль в котором ошибка более 5 раз переписывали подгоняя под стандарты разработки.

Что ж это Вы так плохо работаете? Повышайте качество!

82. ivanov660 4577 11.07.19 13:23 Сейчас в теме

(76)Так можно вообще не ревьювить и не тестировать. Какая получается экономия)
И если такой подход прокатывает (риски и результаты такого подхода "побоку") то можно так и оставить.
Хочешь повысить качество продукта плати деньги.

83. starik-2005 3087 11.07.19 14:04 Сейчас в теме

(82)

Так можно вообще не ревьювить и не тестировать. Какая получается экономия)

Экономии не получается, т.к. стоимость разработки в таких условиях растет из-за:
1. Занятости разработчиков в части исправления ошибок, т.к. вместо разработки они исправляют ошибки.
2. Невозможности решения предоставить функционал, т.к. в нем присутствуют ошибки.
3. Многочисленные повторные поломки.

Тестирование спасает в основном от третьего в части автотестов, т.к. повторные ошибки попадают в покрытие автотестов куда быстрее.

С точки зрения того самого М., основанной не на "я так тут подумал, т.к. пару раз встретил", а на академических исследованиях (читните тот самый "Совершенный код" - там очень много отсылок к именно академическим исследованиям и опыту больших - очень больших - компаний), обычное тестирование тестировщиком функционала решений выявляет 30% багов. Бета-тестирование на заказчике выявляет до 97% багов. Альфа-тестирование у заказчика выявляет что-то между - в районе 60% ошибок, т.к. тестовые сценарии не покрывают весь функционал. При этом ревью кода выявляет 70% ошибок и способствует росту компетенций команды, которая начинает меньше ошибаться, т.к. кейсы с ошибками обсуждаются с примером того, как сделать лучше. Те же примерно 70% дает и парное программирование, основанное на принципе, что одна голова - хорошо, а две - лучше. Т.е. на стадии программирования при парном программировании напарник уже укажет на часть ошибок, которые в итоге не попадут в релиз.

Опять же, это не мое мнение. Но оно достаточно аргументировано со стороны исследователей разработки ПО. Есть команды с другим подходом, но исследователи утверждают, что это стоит дороже.

84. ivanov660 4577 11.07.19 14:28 Сейчас в теме

(83)Это был сарказм)

86. starik-2005 3087 11.07.19 15:40 Сейчас в теме

(84) это было понятно, что не помешало ответить по-существу.

38. ivanov660 4577 10.07.19 09:48 Сейчас в теме

(24) Про внедрение и организационные вопросы могу написать позже если интересно

Ответить

46. for_sale 976 10.07.19 12:53 Сейчас в теме

По куче параметров в функции, которые нужно заменить на одну структуру - с одной стороны я понимаю, о чём речь, с другой стороны на практике читаемость кода падает в ноль, потому что в 99% никто ничего не пишет в комментариях о том, что в этой структуре ожидается. Об этом как раз и нужно было написать - что заменить кучу параметров на одну структуру - это полбеды, нужно ещё вторую половину победить и подробно расписать, что в этой структуре должно быть. Иначе придётся всю процедуру прочесть (каждый раз при её использовании)

47. Pervuy 65 10.07.19 17:39 Сейчас в теме

Добрый день. По поводу пункта 5. Используйте свойство «Пользовательская видимость» я считаю лучше не пользоваться такими штуками в разработке. У меня очень часто пользователи под себя настраивают форму Все действия - Изменить форму и в итоге могу включать те служебные поля или задавать лишние вопросы.

48. ivanov660 4577 10.07.19 19:05 Сейчас в теме

(47)Конечно же включать такой функционал нужно только на время отладки и желательно давать ключевым пользователям. А уж если ваши пользователи такие прошаренные используйте другие механизмы. У всех по разному.

50. starik-2005 3087 11.07.19 09:11 Сейчас в теме

Минусы использования "скрытых" возможностей БСП в том, что эта подсистема может меняться просто нереально, в ней нет обратной совместимости, куча дублирующегося кода, который разработчики в любой момент могут убрать, а функцию перенести в модуль, о котором Вы ничего раньше не знали. Так что не все так однозначно. Хотите отбор в динамическом списке - используйте открытие формы с этим отбором. Хотите условное оформление - сделайте его ручками.

По поводу реквизита в шапке документа, то это хорошо, если документ небольшой, и документов пока немного. Если же их вдруг пару миллионов и они сожержат в себе туеву хучу данных, то нельзя будет так просто взять и добавить новый реквизит в шапку, ибо: 1 - попадете на нереально долгую реструктуризацию, 2 - заполнять этот реквизит неделю будете. Спасает или регистр сведений (он уже есть в типовой - допсведентя или допреквизиты, но это тоже ТЧ, зато уже в отчетах можно юзать - все привязано), или "нарушение" дебильного пункта ЛС под номером 64.сколькототам.

В общем не все так радужно. Но код ревью - это бест практис, так что направление правильное.

56. herfis 513 11.07.19 09:34 Сейчас в теме

(50)

Это реально жесть. БСП по меркам внешнего мира программирования находится в состоянии вечной альфы. "Нет обратной совместимости" - это мягко сказано.
Меня как-то дернуло использовать во внешней универсальной обработке несколько функций из базовой функциональности БСП, которые показались мне удобными, да и смысл изобретать велосипед? В итоге с выходом новых версий БСП мне пришлось несколько раз переписывать обработку, пока в итоге не плюнул и не отвязался от БСП. Ну его в задницу такие "библиотеки". БСП - это просто общий знаменатель текущей генерации типовых конфигураций. Каждую новую генерацию ее кроят как хотят. Минимизация легаси это конечно хорошо. Но не ценой же полного отсутствия обратной совместимости даже для самой невинной функциональности. Было бы круто, если бы хоть на какие-то блоки 1С давало хоть какие-то гарантии обратной совместимости и относилась к ним более бережно. Тогда на них можно было бы хоть как-то опираться.

58. TODD22 19 11.07.19 09:57 Сейчас в теме

(56)Сразу копирую все нужные функции себе в модуль, пару раз завязался на функции БСП. После их изменения всё сломалось. Больше так стараюсь не делать. Что бы потом после очередного обновления не переделывать.

87. for_sale 976 11.07.19 17:01 Сейчас в теме

(58)
Та же фигня. Причём иногда даже копирование не помогает - потому что или внутри скопированных функций что-нибудь тоже вызывается, что потом ломается, или результат, который они дают и который потом отправляется ещё куда-то на доработку, потом не подходит. Потому что БСП она мало того что вечная альфа, так ещё и как макароны расползается по всем частям, взять какой-то механизм как чёрный ящик оттуда невозможно. В общем между "скопировать всю БСП к себе" и "скопировать хотя бы основные нужные и периодически дорабатывать то, что опять поломалось с новой версий" пришлось выбрать второе.

89. starik-2005 3087 12.07.19 10:20 Сейчас в теме

(87)

пришлось выбрать второе

И это должно заставить думать о том, а не сделать ли все самому, а не лезть в БСП за любой фигней. Отборы, оформление, файловые операции - все это можно и без БСП делать, другое дело, что для совместимости с любой платформой придется чуть напрячься, но если платформа юзается одна, то просто увеличить технический долг, ибо он что так, что этак будет расти исходя из роста количества поддерживаемых строк.

90. for_sale 976 12.07.19 10:50 Сейчас в теме

(89)

И это должно заставить думать о том, а не сделать ли все самому, а не лезть в БСП за любой фигней

Для решения сиюминутной задачи - да. Но в общем и целом это путь в никуда. Не мне вам рассказывать, что в любом труЪ-языке библиотеки - это наше всё. В том же ноде.жс любой чих уже есть в какой-то библиотеке, вплоть до библиотек из одной строки (какая-нибудь сумма чисел). Что кстати, недавно привело к катастрофе, но суть не в этом, а в том, что нужно лечить криворуких библиотекарей из 1С, а не клеймить саму библиотечность в целом. Всякие там РазложитьСтрокуВМассив, сообщитьпользователю, сравнить массивы и т.п. - всё это нужно постоянно и каждый раз это писать или даже носить свою библиотечку - так себе варианты. Не говоря уже о более сложных механизмах, вроде адресного классификатора.

91. starik-2005 3087 12.07.19 10:56 Сейчас в теме

(90)

библиотеки - это наше всё

Совершенно соглашусь. Почти в каждом ТруЪ-языке я, лично, юзаю библиотеки и пишу их сам в случае чего. Просто нужно понимать, сколько стоит написать отбор в динамическом списке и сколько стоит поддержка этого, если юзается БСП.

Ежу понятно, что если я хочу отправить письмо или прикрепить файл, то мне проще будет поддерживать функции БСП, а если я хочу установить отбор в ДС или условное оформление, то БСП мне тут не нужна - лучше юзать свой модуль для таких вот трехстрочных функций.

Лично я так рассуждаю: если в функции не больше 10 строк, то стоит ее написать самому. Если больше, то если невозможно вообще отказаться от идеи, и в БСП есть данный функционал, то стоит использовать его.

59. ivanov660 4577 11.07.19 10:20 Сейчас в теме

(50)Согласен. То или иное применение зависит от конкретного проекта.
Обычно, если проект сильно перепилен под себя (т.е. он уже не обновляется), то проблемы смены и развития БСП, будут мало волновать эту команду.

60. starik-2005 3087 11.07.19 10:37 Сейчас в теме

(59)

то проблемы смены и развития БСП, будут мало волновать

Это в том случае, когда не нужна переносимость кода в другую конфигурацию. проблемы возникают при адаптации решений, основанных, например, на УТ 10 или УПП 1.3 в УТ 11 или ЕРП 2 ( с ЗУП 2.5 и 3.Х те же грабли). Поэтому обычно народ дообновляет версию старой системы до актуального релиза, а потом уже оное обновляется дальше. В итоге борьба с переставшим работать функционалом, завязанным на БСП, иногда занимает столько же времени, сколько весь проект целиком.

Но если никто не планирует в обозримом будущем переехать с УТ 10 на УТ 11 или с УПП 1.3 на ЕРП 2.4, то, полагаю, проблем нет.

А иногда особо выдающиеся команды тащат что-то из УТ 11 в УТ 10, а там типовой функционал сильно завязан на БСП, и этот БСП приходится тащить с собой, а с новым БСП, который внутри переплетен сам с собой, старый функционал перестает работать, ибо модули опять же поменялись.

В общем я бы про БСП пунктик пометил как "вредный совет" )))

64. AntonSm 30 11.07.19 10:59 Сейчас в теме

(50) я читаю телеграм-канал про БСП вот этот - @ssl1c.
Там вычитал, что обратная совместимость в БСП есть. Но по определенным правилам.
И что надо в разработке использовать функции из программного интерфейса, а служебный программный не трогать.
Тогда ломаться будет реже, только при смене версии БСП. При чем не при каждой смене версии.
Я так понял, что с БСП не всё так печально, как вы описываете.

Оставьте свое сообщение

E-mail:

Автор:

Владимир Крючков (ivanov660)

Рейтинг: 4577

Для получения уведомлений о новых публикациях автора подключите телеграм бот: Инфостарт бот

Публикация:

№ 1089005

Создание 09.07.19 23:30

Обновление 17.09.19 14:58

Статистика:

Просмотры 18938

Загрузки 0

Рейтинг 120

Комментарии 112

Характеристики:

Код открыт Да

Рубрики Рефакторинг и качество кода

Кому Программист ,
Стажер

Тип файла Нет файла

Платформа Платформа 1С v8.3

Конфигурация Не имеет значения

Операционная система Не имеет значения

Страна Не имеет значения

Отрасль Не имеет значения

Налоги Не имеет значения

Вид учета Не имеет значения

Доступ к файлу Бесплатно (free)