0. ivanov660 1508 04.07.19 17:12 Сейчас в теме

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

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

Перейти к публикации

Комментарии
Избранное Подписка Сортировка: Древо
1. muskul 10.07.19 04:09 Сейчас в теме
мОтбор = новый Структура();
мОтбор.Вставить("Склад",Склад);
// получим остатки
ТаблицаОстатков = РегистрыНакопления.ТоварыНаСкладах.Остатки(ТекущаяДата(),мОтбор,"Склад,Номенклатура","ВНаличии");

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

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

Только вот первый работает во много раз быстрей.
2. nvv1970 10.07.19 06:46 Сейчас в теме
(1) этого не может быть, потому что не может быть.
Подтвердите свои слова техжурналом, планами и текстами запросов.
Если только у вас не отключены текущие итоги и нет миллионов записей после текущей даты.
ivanov660; +1 Ответить
17. muskul 10.07.19 09:10 Сейчас в теме
(2)Увидел такое в средней по объеме базе УТ когда получали долг контрагента по объекту расчета, были удивлены не меньше вашего.
for_sale; +1 Ответить
18. nvv1970 10.07.19 09:15 Сейчас в теме
(17) я не удивлен.
Я говорю про то, что вы чего-то не увидели и не понимаете причин того, что увидели.
Запросы СУБД сравните для начала.
21. muskul 10.07.19 09:20 Сейчас в теме
(18)Зачем причины когда есть факт? прямое обращение к регистру остатков сработало быстрей.
for_sale; +1 Ответить
85. swenzik 11.07.19 15:27 Сейчас в теме
(21) быстрее из-за того что в первом варианте явно указана ТекущаяДата(), добавьте в запрос параметром текущую дату и сравните
88. muskul 12.07.19 02:44 Сейчас в теме
(85)по вашему что я указывал в запросе? Была задача получить долг по документу. что то дернуло посмотреть как будет работать через регистр, увидел что он работает быстрей.
96. tormozit 5512 14.07.19 09:55 Сейчас в теме
(21) На основе результата одного эксперимента опасно делать вывода о преимуществе использованного в нем подхода. Вполне возможно, что твое наблюдение корректно и ты проверил отсутствие других влияющих факторов. Но даже в этом случае, опираясь на такое наблюдение не стоит утверждать, что так будет всегда. Неплохо было бы действительно увидеть запросы к СУБД из техножурнала. Это дало бы на порядок больше понимания. Я бы на твоем месте сделал это хотя бы для повышения собственной квалификации.
99. muskul 15.07.19 03:01 Сейчас в теме
(96)Так в чем проблема взять и проверить. Насчет повышения квалификации полностью согласен. Есть клиент у которого все как у всех а на неболшой ут в клиент сервере поиск отрабатывает то за 2 секунды то за пол минуты думает.
3. json 2334 10.07.19 07:47 Сейчас в теме
Автор, а как получилось, что код-ревью пропустило такое название документа "ЭтапПроизводства2_2" ?
Или у вас это считается нормальной практикой называть метаданные с цифрами на конце?
Summer_13; +1 Ответить
4. ivanov660 1508 10.07.19 07:52 Сейчас в теме
(3)Довольно забавный вопрос)
Пример взят из типовой конфигурации ERP 2.4.6 - это типовой решение от разработчиков компании 1С.
5. CSiER 26 10.07.19 07:56 Сейчас в теме
(3)
ЭтапПроизводства2_2

2_2 в данном случае означает версию конфигурации, которой соответствует логика работы документа.
json; ivanov660; +2 Ответить
7. AlexKo 96 10.07.19 08:08 Сейчас в теме
(3)
Это объект типовой конфигурации ERP 2 1С.
Очень спорное решение архитектора который отвечает в 1С за подсистему производства.
Если в случае ЗаказНаПроизводство2_2 можно натянуто оправдать тем, что оставили объект ЗаказНаПроизводство в конфигурации, то "Этапов производства" не было ранее, были маршрутный листы. Мне не нравится такое решение в типовой.
9. ivanov660 1508 10.07.19 08:19 Сейчас в теме
(7)Чисто теоретически - вдруг архитектор из 1С вдруг ответит в этой ветке)
user811769; +1 Ответить
6. genayo 10.07.19 08:02 Сейчас в теме
С дублированием кода всё не так однозначно, как говорит нам код типовых конфигураций, где этого дублирования выше крыши :))
8. ivanov660 1508 10.07.19 08:17 Сейчас в теме
(6)
1. На мой взгляд однозначно - его надо искоренять. Плюсов его наличия я не вижу.
2.Они между прочем с этим борются (по крайней мере я так предполагаю), но так как объем БСП большой и "сложный" это не быстрый процесс.
acanta; for_sale; +2 Ответить
11. genayo 10.07.19 08:34 Сейчас в теме
(8) Тут был один защитник дублирования - говорил, что единая процедура проведения для нескольких документов зло, вносить изменения на порядок сложнее. Я с ним не совсем согласен, конечно, но преднамеренное избегание дублирования кода должно быть учтено при проектировании архитектуры, мне кажется. Чего с БСП изначально не произошло.
14. ivanov660 1508 10.07.19 08:57 Сейчас в теме
(11)Для исправления дублирования кода существует практика рефакторинга, т.ч. у них еще есть шанс) сделать код лучше.
Мне нравится сайт с классным описанием методик по рефакторингу refactoring.guru
for_sale; +1 Ответить
15. genayo 10.07.19 09:01 Сейчас в теме
(14) Рефакторить архитектуру продукта может быть весьма накладно...
97. tormozit 5512 14.07.19 10:11 Сейчас в теме
(8) Для создания слабо связанных подсистем дублировать код необходимо. Например я объединил свою конфигурацию с подсистемой "СуперУчетАбонентов", в которой есть десятки функций очень похожих (полные или частичные дубли) на уже имеющиеся в моей конфигурации. Конечно разумный программист не станет включать возможность изменения модулей этой подсистемы, чтобы перенаправить вызовы на свои функции и подвергнуть большому риску поломки логику работы этой подсистемы особенно при последующих ее обновлениях. Также и на больших конфигурациях типа ERP разные подсистемы делают разные команды, которые слабо взаимодействуют и потому проводить дедубликацию кода между ними опасно и накладно. Но в целом конечно надо стремиться к дедубликации кода. Если нашел дубль функции в плохо знакомой подсистеме, то по возможности свяжись с ее автором и вместе оцените риски их слияния.
98. ivanov660 1508 14.07.19 10:43 Сейчас в теме
(97)Из-за ограничений 1С и принципа доработки типовых конфигураций, конечно удобнее дублировать код в разумных рамках (чтобы не менять типовой и не париться при изменениях БСП).
Однако, в рамках своих доработок и разработок искоренять дублирование необходимо, а для вопроса согласованности требуется использовать договоренность в рамках некоторого интерфейса (паттерн фасад). Т.е. все эти сложные блоки функционала должны иметь удобный интерфейс - хороший пример far и набор плагинов.
А так получится индусский код, когда каждая команда пишет для себя все что нужно не глядя на то накодили другие. С таким ужасом я впервые (очень давно) столкнулся еще при доработках Битрикса.
10. Lucifer93 69 10.07.19 08:22 Сейчас в теме
Если еще вы не ввели практику использования код-ревью в своей команде, то обязательно реализуйте. Нам без этого было реально сложно.

А что, простите, сложного? Можно увидеть пару аргументов за код-ревью? У нас просто код-ревью не проводится хотелось бы услышать чем становится легче.
12. genayo 10.07.19 08:35 Сейчас в теме
(10) Аргументы для кого? Для разработчика, заказчика продукта, проджект-менеджера?
13. Lucifer93 69 10.07.19 08:38 Сейчас в теме
(12) Хотелось бы услышать комплекс аргументов, просто я, как разработчик, не очень понимаю зачем делать код-ревью. Мне всегда казалось, что достаточно придерживаться стандартов разработки принятых в компании. Хотелось бы понять что даст код-ревью.
16. genayo 10.07.19 09:04 Сейчас в теме
19. Lucifer93 69 10.07.19 09:17 Сейчас в теме
(16) Спасибо, прочитал. Жаль, что автор статьи не указал никаких аргументов в этом пункте, так как не всем необходимо код-ревью.
26. ivanov660 1508 10.07.19 09:34 Сейчас в теме
(13)
(19)
В принципе данное утверждение не должно нуждаться аргументации, т.е. я вас не заставляю, если "созрели", тогда дерзайте. Вот что дало нам (несколько примеров):

1. Это мощная проверка того что делает джун или новичок.
2. Позволяет отсечь ошибки
3. Человек, который знает что его будут проверять, уже пишет лучше.
У нас есть инструкция для код-ревьювера на что он обязательно должен обращать внимание и если эти пункты нарушаются, то задача идет на возврат.
4. Все мы повышаем свою квалификацию.
После многократного разбора ошибок и методик "казусов" стало значительно меньше.
5. Уменьшается эффект bus factor (эффект автобуса)
6. Позволяет выбрать наиболее лучшее решение.
Проверяющий (щие) может не согласиться с реализацией и тогда проблема эскалируется и формируется коллегиальное обсуждение.
33. Lucifer93 69 10.07.19 09:44 Сейчас в теме
(26) Да, я понял, что плюсы есть. Просто в моем случае это не даст особого выхлопа с учетом специфики организации. Спасибо за аргументы, думаю, что их можно включить в первый пункт статьи, чтобы было удобнее.
В принципе данное утверждение не должно нуждаться аргументации, т.е. я вас не заставляю, если "созрели", тогда дерзайте.
Ваши аргументы как раз и дали мне понять, что мы еще не созрели.
36. ivanov660 1508 10.07.19 09:46 Сейчас в теме
(33)Спасибо, попозже поправлю статью с учетом всех замечаний)
63. s_vidyakin 61 11.07.19 10:56 Сейчас в теме
(13) а зачем вы придерживаетесь стандартов разработки? Хотелось бы услышать комплекс аргументов, мне всегда казалось что достаточно просто иметь внятное ТЗ и понимать задачу.
66. TODD22 17 11.07.19 11:02 Сейчас в теме
(63)
а зачем вы придерживаетесь стандартов разработки?

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

ТЗ и понимание задачи это то что надо сделать, стандарты это как надо сделать или как не надо делать и как оформить то что сделал.
Lucifer93; +1 Ответить
77. s_vidyakin 61 11.07.19 11:55 Сейчас в теме
(66) тогда код-ревью это проверка, что кто-то сделал так, как надо сделать, не сделал то, что не надо делать и оформил то что сделал, логично?
78. TODD22 17 11.07.19 12:04 Сейчас в теме
(77)Логично, но это только часть того что даёт код ревью команде.
Это ещё и подтягивание всей команды до одного уровня, общее владение кодом,, изучение лучших и худших практик в одной команде.
starik-2005; +1 Ответить
80. starik-2005 1921 11.07.19 12:37 Сейчас в теме
(77) вообще, логично любой проект защищать. Код-ревью - это защита проекта перед некой приемной комиссией в лице ведущего разработчика или архитектора. Выдвижение формальных требований к оформлению кода, конечно, достаточно бессмысленная процедура (например, именование переменных, функций, псевдонимов в запросах, оформление комментариев или иные моменты, которые не влияют на функционал), а вот функциональные требования к коду, его достаточность в части оптимальности, его влияние на общую производительность решения - это должно быть оценено и, в случае необходимости, подвергнуто сомнению и защищено автором от подобных сомнений путем аргументации. Сам по себе процесс ревью уже даст команде развитие, как один из драйверов прокачки и "софт скиллов", и навыков разработки.
81. TODD22 17 11.07.19 12:44 Сейчас в теме
(80)
например, именование переменных, функций, псевдонимов в запросах, оформление комментариев или иные моменты, которые не влияют на функционал

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

Как то в коде одного коллеги "Если Не МыНеНаСервере Тогда".
20. zqzq 16 10.07.19 09:20 Сейчас в теме
Стоп! Это перебор, необходимо остановиться и сократить количество параметров функции. Вынесите их в одну переменную с типом "Структура":

Произвести уборку путём сгребания мусора под ковёр... По мне так лучше отдельные параметры оставить.
Aggressorak; json; Lucifer93; +3 Ответить
25. json 2334 10.07.19 09:30 Сейчас в теме
(20) очень удачная метафора в данном случае
28. ivanov660 1508 10.07.19 09:38 Сейчас в теме
(20)Не соглашусь, не зря пункт называется "магическое число семь плюс-минус два". Возможно в некоторых случаях требуется пересмотреть архитектурное решение - провести рефакторинг, но не всегда это возможно (при доработках типовых проблематично)
65. s_vidyakin 61 11.07.19 11:02 Сейчас в теме
(20) Нет, т.к. большое количество параметров это повод задуматься, а зачем так много входных данных для процедуры, скорее всего это сваленные в кучу несколько несвязанных операций и надо попробовать разделить их на логичные неделимые шаги с 1-3 параметрами
22. PLAstic 213 10.07.19 09:23 Сейчас в теме
Весь документ пестрит обращениями к значениям перечислений. Почему-то никто из команды код-ревью не прочитал справку, где написано:
ПредопределенноеЗначение(<ИмяПредопределенногоЗначения>)

Описание:

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

Примечание:

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

Раз уж мы говорим про оптимизацию быстродействия...
29. ivanov660 1508 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 213 10.07.19 09:45 Сейчас в теме
(27) В основном да. Что не поощряю - междустрочное выравнивание присвоений.
Объект.ЭтоГрамотноНазванныйРеквизитОбъекта = ИсточникЗаполнения.НекийРеквизит;
Объект.Код                                 = ИсточникЗаполнения.Код;

Глазами теряется, что же мы присваиваем коду.
42. OerlandHue 10.07.19 10:52 Сейчас в теме
(34) а на мой взгляд это одинаково совершенно. Кстати, у нас тимлид запретил делать такое выравнивание. Вот вам и правила кода, подстраивайся под хотелки каждого.
43. ivanov660 1508 10.07.19 11:04 Сейчас в теме
(42)Почему подстраивайся под хотелки каждого, тим лид ввел на всю команду одно правило. Код более будет соответствовать внутри командному стандарту.
Другое дело если две и более команд с разными стандартами, то это жесть. Особенно если задачки будут мигрировать между ними.
49. OerlandHue 11.07.19 06:40 Сейчас в теме
(43) это будет не командный стандарт, а стандарт тим лида. Ему так удобно всю жизнь было и теперь все будут переучиваться писать как он. Потом тимлид сменится и все будут переучиваться за следующим тимлидом. При том, что разницы в чтении кода нет.
Приведу Вам пример из личного. Один руководитель писал всю жизнь запятые при переносе параметров перед параметром. Пришлось переучиваться под его стандарт.
Потом другого члена команды сделали ответственным за правила написания кода. И вуаля, у нас новый стандарт, теперь нужно ставить запятую на строке после параметра. И это кажется, что легко переучиваться, по факту приходится весь код обходить и переставлять запятые.
Круто наверное начитавшись о коннела и прочих РУКОВОДИТЕЛЕЙ программистов внедрять всякие "корпоративные стандарты".
Не круто говорить потом про "команду", как удобно стало "команде" и удивляться, почему мало кто принимает участие в разработке стандартов, почему со скепсисом разработчики относятся к таким вещам.
И этих примеров валом.
Оставьте эти вещи хаотичному порядку. Люди сами начинают писать примерно одинаково, когда работают вместе, без требования от тимлида ставить два пробела после запятой или прочей ерунды. Как это произойдет с пробелами после и до операторов сравнения. Люди сами попросят другого программиста так делать, без Вашего вмешательства и если это действительно мешает работе, то надавят (но на самом деле даже пробелы после и до операторов сравнения может и бесят, но на читабельность не влияют).
kuzyara; tormozit; PLAstic; starik-2005; +4 Ответить
51. starik-2005 1921 11.07.19 09:14 Сейчас в теме
(49) а Вы думали, почему запятые лучше размещать перед параметром? Поверьте, в этом есть гоубокий смысл. MS SQL Men.Studio делает так не от просто потому что)))
OerlandHue; +1 1 Ответить
53. OerlandHue 11.07.19 09:26 Сейчас в теме
(51) Я не знаю про это. У нас есть требование, например, писать текст запроса всегда с отдельной строки, для корректного отображения изменений в git. Я согласен с такими требованиями, потому что это не вкусовщина, это помогает смотреть изменения по проекту, blame и filehistory.
А про запятые в переносе параметров это выглядит конкретно у нас как вкусовщина, потому что никому в команде неизвестен потаенный смысл, зачем так делать. Если Вы скажете, почему лучше делать именно, то я буду благодарен.
(52)
Даже если он неидеален - наличие стандарта лучше, чем его отсутствие.

Я придерживаюсь другого мнения, что чем меньше правил в стандарте, тем лучше. Иначе ваш стандарт никто не запомнит и пользоваться им не будет. А Вы узнаете об этом только через год, когда случайно заглянете в чужой код.
54. starik-2005 1921 11.07.19 09:31 Сейчас в теме
(53) ну тут все на поверхности и пытливому уму должно быть понятно, что пр удалении строки с параметром запятая от предыдущего параметра не останется висеть в конце предыдущей строки. Для запросов выбору умные люди уже давно так делают, а вот для функций и процедур - да, смысла немного...
OerlandHue; +1 1 Ответить
55. OerlandHue 11.07.19 09:33 Сейчас в теме
(54)
ытливому уму должно быть понятно, что пр удалении строки с параметром запятая от предыдущего параметра не останется висеть в конце предыдущей строки. Для запросов выбору умные люди уже давно так делают, а вот для функций и процедур - да, смысла немного...

Действительно удобно :)
Для 1С наверное не подойдет, потому что есть привычка пользоваться конструктором запроса и он такое сотрет.
105. Alien_job 161 31.07.19 19:20 Сейчас в теме
(54) Это ухудшает читаемость, а хороший код принято больше читать чем переписывать
52. herfis 280 11.07.19 09:18 Сейчас в теме
(49)
Оставьте эти вещи хаотичному порядку. Люди сами начинают писать примерно одинаково, когда работают вместе, без требования от тимлида ставить два пробела после запятой или прочей ерунды.

Да-да, конечно же, так всегда и бывает (красненьким). Если тимлиду удается прогнуть остальную команду под свой стиль - это и будет командный стандарт. Даже если он неидеален - наличие стандарта лучше, чем его отсутствие. Трудности "переучивания" преувеличены. Нормальному разрабу не составляет никакого труда следовать навязанному стилю. А вот смешение стилей не просто бесит - оно реально отвлекает и снижает читабельность (и именно поэтому бесит).
ivanov660; +1 Ответить
57. TODD22 17 11.07.19 09:55 Сейчас в теме
(49)
Потом тимлид сменится и все будут переучиваться за следующим тимлидом.

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


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

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


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

С соглашением они начинают это делать гораздо качественнее и быстрее.
herfis; ivanov660; +2 Ответить
30. genayo 10.07.19 09:41 Сейчас в теме
(23) Кстати да, ошибки в именовании переменных, например, и в форматировании, вылавливать на коде ревью нерационально, это точно можно автоматизировать.
35. ivanov660 1508 10.07.19 09:45 Сейчас в теме
40. genayo 10.07.19 10:04 Сейчас в теме
(35) Сам не пользовался, конечно, но беглый гуглинг даёт https://github.com/znsoft/1SCodeAnalyze https://infostart.ru/public/527258/
44. ivanov660 1508 10.07.19 11:05 Сейчас в теме
(40)Хотелось бы уровня pvs-studio.
45. genayo 10.07.19 11:50 Сейчас в теме
(44) У серебряных пулемётчиков вроде есть что-то...
31. ivanov660 1508 10.07.19 09:42 Сейчас в теме
(23)Пример в том, что некоторые коллеги могут не делать никакого форматирования вообще. Возможно вам не встречался конкретный треш, а эта комбинация клавиш приведет код в более или менее наглядный вид.
Не хотите не используйте. Код ревью иногда выполнять не нужно, к примеру, для разовой обработки, которую никогда и никто потом использовать не будет.
24. json 2334 10.07.19 09:27 Сейчас в теме
Автор, опиши еще организационные аспекты код-ревью.

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

Интереснее как такое внедрить в команде:
- кто проводит
- с какой периодичностью
- как исправляете (задачу создаете или еще как)
- как контролируете выполнение исправления (вдруг разработчик просто забил на замечание)
- как боретесь с вкусовщиной (когда у разных разработчиков разное понимание стандартов)
- всех проверяете или выборочно
Fox-trot; AllexSoft; user811769; Artem-B; PLAstic; Dach; OerlandHue; +7 Ответить
32. ivanov660 1508 10.07.19 09:44 Сейчас в теме
(24)По чему не соответствует - по следам код-ревью. Я вкладывал мысль вот мы проводили вебинары внутри команды, составили список и решили поделиться с сообществом теми вопросами, которые обсуждали.
А первый пункт это такой призыв)
Alien_job; +1 Ответить
37. PLAstic 213 10.07.19 09:47 Сейчас в теме
(32) Но осветить вопросы, поставленные оппонентом всё же стОит. Это для многих будет интересно.
user811769; +1 Ответить
39. json 2334 10.07.19 09:52 Сейчас в теме
(32) согласен, не внимательно прочитал.
Заголовок соответствует

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

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

Очень полезно было бы узнать ответы на эти вопросы (возможно в следующей публикации).
41. TODD22 17 10.07.19 10:05 Сейчас в теме
(39)
На каких проектах код-ревью целесообразно применять?

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

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

Из минусов это затраты времени на организацию процесса и на сам процесс, но все минусы перекрываются плюсами, растёт качество продукта, уровень команды, улучшается общее владение кодом, скорость и качество разработки.
К этому не плохо добавлять статические анализаторы кода и тестирование. Но вот с тестированием конечно не всё так однозначно.
PLAstic; genayo; ivanov660; +3 Ответить
61. json 2334 11.07.19 10:50 Сейчас в теме
(41)
Если в команде нет джунов и мидлов. Все ведущие. Любой может сделать любую задачу и сделать хорошо.

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

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

Какая будет польза от организации код-ревью в таких случаях?
62. TODD22 17 11.07.19 10:56 Сейчас в теме
(61)
Каждый знает стандарты и соблюдает.


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

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

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

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

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

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

Сделать хорошо можно двумя способами, с соблюдением соглашений и без соблюдения. "Хорошо" это когда работает. "Соглашения" это требования к оформлению и написанию кода.
70. json 2334 11.07.19 11:14 Сейчас в теме
(62)
(67)
Мы возможно друг друга не понимаем.

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

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

Дешевле исправить их во время обнаружения или я не прав?
74. TODD22 17 11.07.19 11:29 Сейчас в теме
(70)
Ведь код-ревью, это другими словами, выискивание косяков.

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

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

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

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

Да и вообще код ревью способствует так сказать "сплачённости" коллектива. Превратите "код ревью" в небольшой пятничный "тим билдинг" и профит не заставит себя долго ждать.
92. PLAstic 213 12.07.19 11:05 Сейчас в теме
(74) А я же правильно понимаю, что КР даёт ещё достаточно глубокое погружение проверяющего в реализуемые механизмы проверяемого? Т.е. позже проверяющий более активно сможет использовать механизмы проверяемого и в своей работе.

Просто, у нас есть такая проблема, что один пишет полезную для других функцию или механизм, а другие про это не знают. При реализации подсистем, конечно, оповещение по почте идёт, но не будешь же рассылку делать при реализации каждой процедуры/функции?
93. TODD22 17 12.07.19 11:12 Сейчас в теме
(92)
Просто, у нас есть такая проблема, что один пишет полезную для других функцию или механизм, а другие про это не знают.

:)

Это уже надо как то организовывать что то типа "общей библиотеки" с документацией и обязательным ознакомлением.
75. TODD22 17 11.07.19 11:31 Сейчас в теме
(70)
Дешевле исправить их во время обнаружения или я не прав?

Это зависит от того дешевле ли их исправить во время обнаружения. Исправлять что то в базе которая работает 24*7 и у тебя в неделю одно окно на 3 часа для установки обновлений может быть очень не дёшево.
67. starik-2005 1921 11.07.19 11:03 Сейчас в теме
(61)
Какая будет польза от организации код-ревью в таких случаях?
На мой личный взгляд, код-ревью помогает при решении задачи дополнительно разработчику подумать о том, что когда-то это будут смотреть не менее умные люди, чем он, а, как всем известно, "и на старуху бывает проруха".

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

Т.е. всегда есть два потока - восходящий и нисходящий, поток ошибок и интересных решений. Код-ревью - отличный транслятор опыта в команду.
PLAstic; acanta; +2 Ответить
69. AntonSm 25 11.07.19 11:12 Сейчас в теме
(67) вот тут небезызвестный Лустин говорит, что кодревью это дорого, очень дорого.
Т.е. вы считаете, что кодревью стоит затрат, потраченных на него, в любом случае?
72. starik-2005 1921 11.07.19 11:20 Сейчас в теме
(69) небезызвестный Макконнелл говорит, что код-ревью в 4 раза дешевле тестирования и в 2 раза эффективнее. Даже и не знаю, кому из этих авторитетов верить...
76. AntonSm 25 11.07.19 11:32 Сейчас в теме
(72)
Даже и не знаю, кому из этих авторитетов верить...


Вроде оба дядьки умные.
Но Лустин говорит про сегодняшние реалии.
Еще и в контексте применения автотестов всяких.
У Макконнелла про автотесты было что-то, не подскажите?
Непонятно, когда эти автотесты появились, до или после подсчета стоимости тестирования.
79. starik-2005 1921 11.07.19 12:29 Сейчас в теме
(76)
У Макконнелла про автотесты было что-то, не подскажите?
Непонятно, когда эти автотесты появились, до или после подсчета стоимости тестирования.
У Макконнелла про тестирование было то, что это дорого. На Боинге тестирование стоило в 8 раз дороже, чем ревью кода, но мы то знаем, где сейчас Боинг с их МАХ (правда, это только с ним такие проблемы, и я лучше на самолете производства Боинг полечу, чем на суперджете, где, предположу, толком нет ни того, ни другого).

Автотесты сейчас почти во всех проектах есть, но их наличие позволяет найти в большинстве своем те проблемы, которые уже были когда-то обнаружены, при том не единожды. И есть у Макконнелла мнение, основанное на исследованиях разработки ПО, что тестирование получается сильно дороже экстремальной разработки с использованием парного программирования и гибких подходов, ревью кода в которых считается неотьемлемой частью процесса постоянного улучшения качества.
94. qwinter 596 12.07.19 15:49 Сейчас в теме
(79)
На Боинге тестирование стоило в 8 раз дороже, чем ревью кода
Так на МАХ было именно КР, и минимум тестов. И модуль в котором ошибка более 5 раз переписывали подгоняя под стандарты разработки.
95. starik-2005 1921 12.07.19 17:49 Сейчас в теме
(94)
И модуль в котором ошибка более 5 раз переписывали подгоняя под стандарты разработки.
Что ж это Вы так плохо работаете? Повышайте качество!
82. ivanov660 1508 11.07.19 13:23 Сейчас в теме
(76)Так можно вообще не ревьювить и не тестировать. Какая получается экономия)
И если такой подход прокатывает (риски и результаты такого подхода "побоку") то можно так и оставить.
Хочешь повысить качество продукта плати деньги.
83. starik-2005 1921 11.07.19 14:04 Сейчас в теме
(82)
Так можно вообще не ревьювить и не тестировать. Какая получается экономия)
Экономии не получается, т.к. стоимость разработки в таких условиях растет из-за:
1. Занятости разработчиков в части исправления ошибок, т.к. вместо разработки они исправляют ошибки.
2. Невозможности решения предоставить функционал, т.к. в нем присутствуют ошибки.
3. Многочисленные повторные поломки.

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

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

Опять же, это не мое мнение. Но оно достаточно аргументировано со стороны исследователей разработки ПО. Есть команды с другим подходом, но исследователи утверждают, что это стоит дороже.
84. ivanov660 1508 11.07.19 14:28 Сейчас в теме
86. starik-2005 1921 11.07.19 15:40 Сейчас в теме
(84) это было понятно, что не помешало ответить по-существу.
38. ivanov660 1508 10.07.19 09:48 Сейчас в теме
(24) Про внедрение и организационные вопросы могу написать позже если интересно
Aggressorak; dhurricane; testnv0; Krio2; hlop11; Artem-B; genayo; json; PLAstic; +9 Ответить
46. for_sale 764 10.07.19 12:53 Сейчас в теме
По куче параметров в функции, которые нужно заменить на одну структуру - с одной стороны я понимаю, о чём речь, с другой стороны на практике читаемость кода падает в ноль, потому что в 99% никто ничего не пишет в комментариях о том, что в этой структуре ожидается. Об этом как раз и нужно было написать - что заменить кучу параметров на одну структуру - это полбеды, нужно ещё вторую половину победить и подробно расписать, что в этой структуре должно быть. Иначе придётся всю процедуру прочесть (каждый раз при её использовании)
sh_max; json; Fox-trot; ivanov660; +4 Ответить
47. Pervuy 31 10.07.19 17:39 Сейчас в теме
Добрый день. По поводу пункта 5. Используйте свойство «Пользовательская видимость» я считаю лучше не пользоваться такими штуками в разработке. У меня очень часто пользователи под себя настраивают форму Все действия - Изменить форму и в итоге могу включать те служебные поля или задавать лишние вопросы.
48. ivanov660 1508 10.07.19 19:05 Сейчас в теме
(47)Конечно же включать такой функционал нужно только на время отладки и желательно давать ключевым пользователям. А уж если ваши пользователи такие прошаренные используйте другие механизмы. У всех по разному.
50. starik-2005 1921 11.07.19 09:11 Сейчас в теме
Минусы использования "скрытых" возможностей БСП в том, что эта подсистема может меняться просто нереально, в ней нет обратной совместимости, куча дублирующегося кода, который разработчики в любой момент могут убрать, а функцию перенести в модуль, о котором Вы ничего раньше не знали. Так что не все так однозначно. Хотите отбор в динамическом списке - используйте открытие формы с этим отбором. Хотите условное оформление - сделайте его ручками.

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

В общем не все так радужно. Но код ревью - это бест практис, так что направление правильное.
for_sale; +1 Ответить
56. herfis 280 11.07.19 09:34 Сейчас в теме
(50)
Минусы использования "скрытых" возможностей БСП в том, что эта подсистема может меняться просто нереально, в ней нет обратной совместимости, куча дублирующегося кода, который разработчики в любой момент могут убрать

Это реально жесть. БСП по меркам внешнего мира программирования находится в состоянии вечной альфы. "Нет обратной совместимости" - это мягко сказано.
Меня как-то дернуло использовать во внешней универсальной обработке несколько функций из базовой функциональности БСП, которые показались мне удобными, да и смысл изобретать велосипед? В итоге с выходом новых версий БСП мне пришлось несколько раз переписывать обработку, пока в итоге не плюнул и не отвязался от БСП. Ну его в задницу такие "библиотеки". БСП - это просто общий знаменатель текущей генерации типовых конфигураций. Каждую новую генерацию ее кроят как хотят. Минимизация легаси это конечно хорошо. Но не ценой же полного отсутствия обратной совместимости даже для самой невинной функциональности. Было бы круто, если бы хоть на какие-то блоки 1С давало хоть какие-то гарантии обратной совместимости и относилась к ним более бережно. Тогда на них можно было бы хоть как-то опираться.
for_sale; +1 Ответить
58. TODD22 17 11.07.19 09:57 Сейчас в теме
(56)Сразу копирую все нужные функции себе в модуль, пару раз завязался на функции БСП. После их изменения всё сломалось. Больше так стараюсь не делать. Что бы потом после очередного обновления не переделывать.
for_sale; +1 Ответить
87. for_sale 764 11.07.19 17:01 Сейчас в теме
(58)
Та же фигня. Причём иногда даже копирование не помогает - потому что или внутри скопированных функций что-нибудь тоже вызывается, что потом ломается, или результат, который они дают и который потом отправляется ещё куда-то на доработку, потом не подходит. Потому что БСП она мало того что вечная альфа, так ещё и как макароны расползается по всем частям, взять какой-то механизм как чёрный ящик оттуда невозможно. В общем между "скопировать всю БСП к себе" и "скопировать хотя бы основные нужные и периодически дорабатывать то, что опять поломалось с новой версий" пришлось выбрать второе.
89. starik-2005 1921 12.07.19 10:20 Сейчас в теме
(87)
пришлось выбрать второе
И это должно заставить думать о том, а не сделать ли все самому, а не лезть в БСП за любой фигней. Отборы, оформление, файловые операции - все это можно и без БСП делать, другое дело, что для совместимости с любой платформой придется чуть напрячься, но если платформа юзается одна, то просто увеличить технический долг, ибо он что так, что этак будет расти исходя из роста количества поддерживаемых строк.
90. for_sale 764 12.07.19 10:50 Сейчас в теме
(89)
И это должно заставить думать о том, а не сделать ли все самому, а не лезть в БСП за любой фигней

Для решения сиюминутной задачи - да. Но в общем и целом это путь в никуда. Не мне вам рассказывать, что в любом труЪ-языке библиотеки - это наше всё. В том же ноде.жс любой чих уже есть в какой-то библиотеке, вплоть до библиотек из одной строки (какая-нибудь сумма чисел). Что кстати, недавно привело к катастрофе, но суть не в этом, а в том, что нужно лечить криворуких библиотекарей из 1С, а не клеймить саму библиотечность в целом. Всякие там РазложитьСтрокуВМассив, сообщитьпользователю, сравнить массивы и т.п. - всё это нужно постоянно и каждый раз это писать или даже носить свою библиотечку - так себе варианты. Не говоря уже о более сложных механизмах, вроде адресного классификатора.
91. starik-2005 1921 12.07.19 10:56 Сейчас в теме
(90)
библиотеки - это наше всё
Совершенно соглашусь. Почти в каждом ТруЪ-языке я, лично, юзаю библиотеки и пишу их сам в случае чего. Просто нужно понимать, сколько стоит написать отбор в динамическом списке и сколько стоит поддержка этого, если юзается БСП.

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

Лично я так рассуждаю: если в функции не больше 10 строк, то стоит ее написать самому. Если больше, то если невозможно вообще отказаться от идеи, и в БСП есть данный функционал, то стоит использовать его.
for_sale; +1 Ответить
59. ivanov660 1508 11.07.19 10:20 Сейчас в теме
(50)Согласен. То или иное применение зависит от конкретного проекта.
Обычно, если проект сильно перепилен под себя (т.е. он уже не обновляется), то проблемы смены и развития БСП, будут мало волновать эту команду.
60. starik-2005 1921 11.07.19 10:37 Сейчас в теме
(59)
то проблемы смены и развития БСП, будут мало волновать
Это в том случае, когда не нужна переносимость кода в другую конфигурацию. проблемы возникают при адаптации решений, основанных, например, на УТ 10 или УПП 1.3 в УТ 11 или ЕРП 2 ( с ЗУП 2.5 и 3.Х те же грабли). Поэтому обычно народ дообновляет версию старой системы до актуального релиза, а потом уже оное обновляется дальше. В итоге борьба с переставшим работать функционалом, завязанным на БСП, иногда занимает столько же времени, сколько весь проект целиком.

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

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

В общем я бы про БСП пунктик пометил как "вредный совет" )))
64. AntonSm 25 11.07.19 10:59 Сейчас в теме
(50) я читаю телеграм-канал про БСП вот этот - @ssl1c.
Там вычитал, что обратная совместимость в БСП есть. Но по определенным правилам.
И что надо в разработке использовать функции из программного интерфейса, а служебный программный не трогать.
Тогда ломаться будет реже, только при смене версии БСП. При чем не при каждой смене версии.
Я так понял, что с БСП не всё так печально, как вы описываете.
68. starik-2005 1921 11.07.19 11:05 Сейчас в теме
(64)
Тогда ломаться будет реже
Это невозможно спрогнозировать. Иногда может сломаться что-то, что сломаться в принципе не должно бы было. особенно если функционал предполагается использовать на разных версиях разных типовых решений.
71. AntonSm 25 11.07.19 11:15 Сейчас в теме
(68) БСП сейчас (по заявлению руководителя евойной разработки) тестами покрыта чуть менее, чем полностью.
БСП крутая стала.

Хорошо бы пример вот такого:

(68)
Иногда может сломаться что-то, что сломаться в принципе не должно бы было.
Оставьте свое сообщение
Новые вопросы с вознаграждением
Автор темы объявил вознаграждение за найденный ответ, его получит тот, кто первый поможет автору.

Вакансии

Программист 1С
Санкт-Петербург
зарплата от 150 000 руб.
Полный день

Программист 1С
Тюмень
зарплата от 70 000 руб.
Полный день

Программист 1С
Москва
зарплата от 150 000 руб. до 150 000 руб.
Полный день

Консультант 1С
Нижний Новгород
зарплата до 100 000 руб.
Полный день

Программист стажер 1С
Нижний Новгород
зарплата от 30 000 руб.
Полный день