Управление качеством кода

22.07.2019 33 Comments

Математика и алгоритмы, Разработка

, , , ,

Related Posts

33 Comments

  1. Evil Beaver

    Охрененно! Аффтар, пешы есчо, хорошо же получается!

    Reply ↓
  2. mrXoxot

    Очень здорово!

    Пусть все получиться!

    Reply ↓
  3. artbear

    (0) >Все любят хорошую документация

    Опечатка.

    Reply ↓
  4. artbear

    (0) Отличная статья. Радует, что все больше разработчиков/команд обращают внимание на качество своей работы — статический анализ кода, автоматические тесты, CI, DevOps и вот это вот все.

    Reply ↓
  5. artbear
    Проверяются только модули со снятыми замками.

    Если замок стоит — это не наша проблема.

    Вот тут на самом деле непросто.

    Да, такое решение упрощает жизнь, скорость анализа, количество замечаний и т.п.

    Но самом деле модули на замке (модули от вендора) — это также проблема «наша» (команды, разработчиков), если эти модули используются в бою.

    Ведь если в продуктиве возникнет ошибка в таком модуле, бизнес будет обращаться к нам, как к разработчикам/внедренцам системы, а не к вендору, верно 🙂 ?

    Команде проекта нужно решить, нужно ли все-таки учитывать такие модули или нет.

    Для сложных систем для наших клиентов я советую следующее:

    — учитывать модули на замке, а не отбрасывать

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

    — вдруг все-таки вендор серьезно ошибся, ведь и такое бывает.

    Reply ↓
  6. Scorpion4eg

    Жаль что никто кроме EDT напрямую с внешними обработками не работает.

    АПК от слова совсем. Sonar более менее пережевывает… А ведь во внешних печатных формах можно столько всего на плохокодить.

    Reply ↓
  7. olegtymko

    (6) Паковать обработки / отчеты в cf/cfe и прогонять в EDT, АПК и заливать все в SonarQube. Тогда весь плохокод будет на лицо.

    Reply ↓
  8. artbear

    (6) Почему «никто» ? наш платный плагин для SonarQube (от Пули) прекрасно работает.

    и в VSC можно немедленно получать результаты, если кодируешь код в VSC.

    давно проверяю инструмент Ванесса-АДД, построенный как раз на внешних обработках.

    Reply ↓
  9. artbear

    (7) Костыль ИМХО. но ради качества на что не пойдешь 🙂

    Reply ↓
  10. theshadowco

    (8) токо в формате edt увы работает неахти

    Reply ↓
  11. Scorpion4eg

    (8) Не спорю, классно работает! Но когда мы его тестировали была проблема с «Возможно неиспользуемая процедура» для обработчиков элементов формы. Решили уже?

    Reply ↓
  12. Fox-trot

    (11) достаточно добавить операнд «Экспорт», хотя костыль конечно

    Reply ↓
  13. artbear

    (11) Да, эта проблема решена в конце прошлого года или начале этого

    Reply ↓
  14. vlad.frost
    Правило бойскаута

    Отдельно выделю — Оставляйте код чище, чем застали. Сделайте чуточку лучше тот код, который трогаете при кодировании текущей задачи.

    SonarQube, кстати, позволяет этому правилу следовать с чуть меньшими рисками:

    во-первых, можно заранее, перед выполнением задачи, посмотреть, какие есть в коде нарушения стандартов;

    во-вторых, после всех правок убедиться, что все нарушения-таки исправлены.

    Reply ↓
  15. user614660_dajigin

    Все таки есть сомнения по поводу «Иначе». Очень много конструкций таких типов:

    Если КакойТоПризнак = ПервоеЗначение Тогда

    ВыполнитьДополнительныеДействия1;

    ИначеЕсли КакойТоПризнак = ВтороеЗначение Тогда

    ВыполнитьДополнительныеДействия2;

    КонецЕсли;

    …Продолжаем выполнение по плану: действия, которые должны выполняться для ПервоеЗначение, ВтороеЗначение и все последующие….

    Что в этом случае? Пустой «Иначе» делать? Какой в нем смысл? Исключения выкидывать нельзя, т.к. «непопадание» в условия это не ошибка.

    Reply ↓
  16. Labotamy

    (15)В этих случаях диагностика ничего вам не скажет. Диагностика срабатывает на Если ИначеЕсли без Иначе.

    Reply ↓
  17. user614660_dajigin

    Уже понял косяк)) Исправил вопрос))

    Reply ↓
  18. Labotamy

    (17)Посмотрите комментарии к статье Олега. Там весёлый холиварчик на эту тему вышел.

    Reply ↓
  19. user614660_dajigin

    (18) Ага, спасибо, нашел)

    Reply ↓
  20. artbear

    (15) Да, эта диагностика холиварна и дает много срабатываний.

    Есть важный принцип статического анализа — количество ложных срабатываний должно быть минимальным или их вообще не должно быть.

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

    Поэтому мы для своего платного плагина (от Пули) все еще думаем над необходимостью введения этого правила.

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

    Reply ↓
  21. olegtymko

    (20) Опять же это очень холиварная тема.

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

    К примеру:

    Дан какой-нибудь произвольный обмен с партнером через FTP в их кастомном формате. В коде есть условие, которое определяет что делать с документом

    ДД = Новый ДвоичныеДанные(); // какие-то двоичные данные
ВидФайла = «ARRAIV»; // вид файла, который прочитали из именя файла

Если ВидФайла = «PICK» Тогда
ЗагрузитьКакРасходнуюНакладную(ДД);
ИначеЕсли ВидФайла = «ORDER» Тогда
ЗагрузитьКакЗаказ(ДД);
ИначеЕсли Видфайла = «STOCK» Тогда
ЗагрузитьКакРеестрОстатков(ДД);
КонецЕсли;

    Показать

    Если не сообщить о виде файла, что он не поддерживается, то проблема может жить очень долго и никто об этом не узнает. В этом случае можно код допилить так:

    Если ВидФайла = «PICK» Тогда
ЗагрузитьКакРасходнуюНакладную(ДД);
ИначеЕсли ВидФайла = «ORDER» Тогда
ЗагрузитьКакЗаказ(ДД);
ИначеЕсли Видфайла = «STOCK» Тогда
ЗагрузитьКакРеестрОстатков(ДД);
Иначе
// или исключение (при условии что выше по уровню стоит попытка и отлавливает все исключения)
ВызватьИсключение(СтрШаблон(«Вид файла %1 не поддерживается»));
// или соообщение в какой нибудь лог
Логирование.Ошибка(«Обмен.СамыйЛучшийПарнер», СтрШаблон(«Вид файла %1 не поддерживается»));
КонецЕсли;

    Показать

    Да, ложные срабатывания бывают. Когда в алгоритме понятно, что если условия не выполнились то и делать ничего не надо. Но есть способ отключить проверку в определенном блоке кода.

    Reply ↓
  22. kalimehtar

    (15)

    Пустой «Иначе» делать? Какой в нем смысл?

    Смысл в том, что ты явно указал любому читающему, что ветка Иначе пропущена не случайно. Также как в проверках Си «while (c = getchar()) » считается ошибкой, а «while ((c = getchar()))» нормальный код. Вторая пара скобок указывает, что присваивание там осознанно, а не перепутали со сравнением «==».

    Reply ↓
  23. Tavalik

    Крутая статья. Спасибо.

    Reply ↓
  24. Scorpion4eg

    (20) Никогда не понимал почему холиварная. Для себя давно завел правило: не знаешь что писать в иначе, пишу ВызватьИсключение «Неожиданный вариант».

    Исключение — обработка оповещения в формах

    Reply ↓
  25. Ndochp

    (22) Только АФАИР влетаешь в диагностику » в блоке нет кода»

    Reply ↓
  26. Ndochp

    (24) Ну вот смотри, у тебя есть что-то, что работает с десятком видов документов и в 8 в реквизите то, что тебе надо, в девятом префикс «ДД.», в десятом — суффикс «.Ик»

    вот ты и пишешь:

    Если ВидФайла = «PICK» Тогда
ПараметрСтрокой = Сред(ПараметрСтрокой, 4);
ИначеЕсли ВидФайла = «ORDER» Тогда
ПараметрСтрокой = Лев(ПараметрСтрокой, СТрДлина(ПараметрСтрокой) — 3);
КонецЕсли;
    Reply ↓
  27. Scorpion4eg

    (26) Это понятно. Но что ты будешь делать, если ВНЕЗАПНО (а это всегда внезапно) придет новый ВидФайла? Сколько времени потребуется на поиск ошибки?

    А так добавляешь строку:

    Иначе

    ВызватьИсключение «Неожиданное значение переменной ВидФайла» + ВидФайла;

    КонецЕсли;

    Reply ↓
  28. Ndochp

    (27)У меня для 8 видов все хорошо, для двух — проблемы. Скорее всего с одиннадцатым все будет тоже хорошо. А если не будет, то я об этом не забуду.

    Или вы предлагаете сделать еще один блок с видами файла 1..8? А если я точно знаю, что у меня 2 исключения, но не уверен, сколько основных?

    Типа уникального перемещения между складами — единственный документ с двумя складами. Вы знаете все документы в конфигурации с реквизитом «Склад»?

    Reply ↓
  29. Labotamy

    (28) А все коллеги знают и помнят эти «нюансы»? И те кто будут после, тоже?

    Reply ↓
  30. Ndochp

    А куда деваться? или предлагаете перед каждым использованием реквизита «Склад» в экспортных процедурах модулей складской подсистемы делать проверку на тип документа и выбрасывать исключение для новых типов?

    Reply ↓
  31. FrancuzbyGmailcom

    Кажется это «содрано» из книги «Совершенный код» 😉

    Reply ↓
  32. Labotamy

    (31) Вероятней из «чистый».

    Reply ↓
  33. Stepa86

    (32) Про стандарты 1С у Макконнелла все же лучше написано

    Reply ↓

Leave a Comment

Ваш адрес email не будет опубликован. Обязательные поля помечены *