Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Роман Просин «Code Review — искусство развивать»

DotNetRu
February 25, 2020

Роман Просин «Code Review — искусство развивать»

Говорят, повторенье мать ученья. Поверим на слово и рассмотрим тонкости искусства инспекции кода на примере языка C# так, как будто никогда о них не слышали.

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

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

DotNetRu

February 25, 2020
Tweet

More Decks by DotNetRu

Other Decks in Programming

Transcript

  1. Меня зовут Просин Роман. Мой путь разработчика начался более 7

    лет назад 2 из которых я работаю разработчиком в ICDB Team Райффайзенбанка. Всем привет 2 Почему Code Review? «Вы должны знать чему противостоите. Вы должны быть готовы!»
  2. Код под увеличительным стеклом 9 [CakeMethodAlias] public static string Parameter(

    this ICakeContext context, string name, string prefix = "") => Parameter(context, name, prefix); [CakeMethodAlias] public static T Parameter<T>( this ICakeContext context, string name, T @default, string prefix = "") { … } StackOverflowException!
  3. [CakeMethodAlias] public static T Parameter<T>( this ICakeContext context, string name,

    T @default, string prefix = "") { context.ThrowIfNull(nameof(context)); name.ThrowIfNullOrWhiteSpace(nameof(name)); return context.HasArgument(name) ? context.Argument(name, @default) : context.HasEnvironmentVariable(name) ? context.EnvironmentVariable($"{prefix}{name}", @default) : @default; } Код под увеличительным стеклом Высокая когнитивная сложность!
  4. Что сказать Марку? [CakeMethodAlias] public static string Parameter( this ICakeContext

    context, string name, string prefix = "") => Parameter(context, name, prefix); Это работать не будет... Тут же будет StackOverflowException! Марк, почитай про бесконечную рекурсию. Если хочешь её применить, тогда нужно реализовать условие возврата, иначе ты получишь StackOverflowException! Ну ты и … Это же не возможно читать. Исправь это! Марк, в командных соглашениях есть пункт запрещающий объединять тернарные операторы в цепочки. Их сложно воспринимать. Замени на блоки, пожалуйста.
  5. Плохой обзор кода x Оценка; x Критика в адрес автора;

    x Безапелляционное утверждение; x Не информативный комментарий;
  6. 13

  7. Хвалите 16 ✓ Отличная работа! Молодец. ✓ О.. Отлично придумано.

    Теперь я могу достать параметр одной строкой. Красавчик! ✓ Классная фича. Спасибо!
  8. Ремонтопригодность 19 public void DoSomething( int param1, int param2, int

    param3, string param4, long param5) { ... } public void DoSomething(int param1, int param2, int param3, string param4) { ... }
  9. Ремонтопригодность 20 var x = personList .OrderBy(person => person.Age) .OrderBy(person

    => person.Name) .ToList(); var x = personList .OrderBy(person => person.Age) .ThenBy(person => person.Name) .ToList(); • Коллекция вернётся отсортированной по имени • Не хватает тестов
  10. int Pow(int num, int exponent) { num *= Pow(num, exponent

    - 1); return num; } int Pow(int num, int exponent) { if (exponent > 1) { num *= Pow(num, exponent - 1); } return num; } Надёжность StackOverflowException!
  11. public class SqlInjection : Controller { // GET /SqlInjection/Authenticate public

    IActionResult Authenticate(string user) { var query = "SELECT * FROM Users WHERE Username = '" + user + "'"; var userExists = context.Users.FromSql(query).Any(); user = "' or 1=1 or ''='"; return Content(userExists ? "success" : "fail"); } } Безопасность
  12. Безопасность public class SqlInjection : Controller { //… // GET

    /SqlInjection/Authenticate public IActionResult Authenticate(string user) { var query = "SELECT * FROM Users WHERE Username = {0}"; var userExists = context.Users.FromSql(query, user).Any(); return Content(userExists ? "success" : "fail"); } }
  13. Какие могут быть сценарии Затягивание и блокировка или интеллектуальная усталость

    Поспешность в инспекции кода Внешние рецензенты Ложные дефекты. Траты времени на исправления того, что работает.
  14. Затягивание и блокировка 0 1 2 3 4 5 6

    7 8 0 20 40 60 80 100 Найдено дефектов Время (минут) Эффективность поиска 26
  15. 0 20 40 60 80 100 120 140 160 0

    200 400 600 800 1000 1200 1400 Плотность дефектов (KLOC) Скорость рецензирования (LOC/час) Зависимость плотности дефектов от скорости рецензирования Поспешность 27
  16. Внешние рецензенты 28 85 48 0 10 20 30 40

    50 60 70 80 90 Без самоанализа С самоанализом Средняя плотность дефектов (дефекты/KLOC) Влияние самоанализа на плотность дефектов
  17. Ложные дефекты 29 95 96 98 97 95 92 94

    93 99 91 92 97 94 5 4 2 3 5 8 6 7 1 9 8 3 6 86 88 90 92 94 96 98 100 102 Дефекты обнаруженные в фазе инспектирования В ходе самостоятельного анализа В рамках встреч
  18. Editorconfig • Стили наименования • Рекомендации по написанию кода .NET

    • Рекомендации по написанию кода C# • Правила форматирования C# • Стили именования
  19. 36 Пишем Cake скрипты. Cake - это кроссплатформенная система автоматизации

    сборки с C# DSL Запуск Cake Scripts на CI сервере Cake запускает NUnit Console Runner и тесты Cake запускает Nuget и упаковывает артефакты в пакеты. Cake отправляет артефакты куда скажете Управляем версиями через GIT
  20. Простой скрипт Task("Build") .Does(() => { var solution = GetFiles("../**/Cake.Addins.sln").FirstOrDefault();

    if(solution is null) { throw new CakeException("Solution not found"); } MSBuild(solution); }); RunTarget(target); string target = Argument(nameof(target), "Analysis");
  21. Переменная X не инициализирована int x; int y = x

    + 2; Статический анализ кода
  22. 40

  23. Настройка #addin nuget:?package=Cake.Sonar&version=1.1.18 #tool nuget:?package=MSBuild.SonarQube.Runner.Tool Setup(ctx => { Information("Running setup...");

    sonarSettings = new SonarBeginSettings() { Url = "https://sonarcloud.io", Login = "f6509d3b36c7d016beace4046256488d844e9bec", Key = "code.review", Name = "Code Review", Version = "1.0.0.0", UseCoreClr = true, }; }); 41
  24. Добавление задачи 42 Task("InitializeSonar") .Does(() => { SonarBegin(sonarSettings); }); Task("Analysis")

    .IsDependentOn("InitializeSonar") .IsDependentOn("Build") .Does(() => { SonarEnd(sonarSettings.GetEndSettings()); });
  25. 43

  26. 44

  27. ☺ Не нужно явно запускать анализаторы ☺ Не нужно ждать

    сборки ☺ Анализаторы на основе Roslyn выполняются асинхронно, во время вашей работы. ☺ Объективность ☺ Не вызывает сомнений Преимущества Roslyn анализаторов 48
  28. Полезное 50 ▪ Орам Э. Идеальная разработка ПО. Рецепты лучших

    программистов. / Орам Э., Уилсон Г. - СПб.: Питер 2012.— 592 с. ▪ https://cakebuild.net/ ▪ https://sonarcloud.io/