Всем добрый день. Некоторое время назад я выступал перед студентами на тему "Что мы ожидаем от хорошего кода" и решил продублировать ее тут. В процессе перевода текст несколько поменялся, но суть осталась прежней. Статья получилась простая (и безусловно не полная), но рациональное зерно тут есть.
Код должен работать
Давайте поговорим о том, что мы ожидаем от кода. Ну, для начала, он должен работать.
Звучит, конечно, очевидно, но каждый из нас когда-то пытался или успешно запушил код, который даже не собирается, так что не смейтесь. Второй момент — код должен корректно работать с некорректными ситуациями. То есть ловить ошибки. Но давайте пока вернемся к первому пункту и немного поговорим об этом.
Периодически мне попадает задача, которую я понятия не имею как делать. То есть вообще (я стараюсь смотреть вокруг и постоянно пробую что-то новое). И тут меня сразу тянет писать какие-то абстракции, какую-то инфраструктуру, чтобы оттянуть момент реальной работы. Так вот, это неправильно. Код должен работать. Я знаю, что я повторяюсь, но это важный момент. Если вы не знаете, как решать задачу — не надо бросаться создавать интерфейсы, модули и вот это все. Это плохая идея, которая закончится тем, что у вас истечет время, а вы никуда не продвинетесь. Запомните, плохо работающий код, во много раз лучше хорошего, но не работающего кода.
Есть старая притча про две софтверные компании, которые делали один и тот же продукт. Первая делала абы как, но первой вышла на рынок, а вторая делала все идеально и опоздала. В результате, первая кампания успела рынок завоевать и выкупила вторую компанию. Это немного про другое, но основная идея все равно та же. Сначала решаем проблему, потом делаем код красивым.
В общем, сперва сделайте работающий прототип. Пусть он будет хромой, кривой и несчастный, зато, когда с вас спросят — вы сможете сказать что решение уже есть, осталось его интегрировать. И перепишите его как надо. Это можно попробовать выразить такой максимой — если вы знаете как делать задачу — делайте ее хорошо. Если не знаете — сначала решите ее хоть как-то.
И тут есть важный момент. Я бы хотел, чтобы вы поняли. Это не призыв писать плохой код. Код должен быть хороший. Это призыв к First Thing First — сначала код работает, потом рефакторится.
Теперь поговорим про Shit Happens. Итак, код у нас есть, он даже работает. Вернее, "работает". Давайте посмотрим на простой пример:
public string Do(int x)
{
using (WebClient xx = new WebClient())
{
return xx.DownloadString("https://some.super.url");
}
}
Это замечательный пример "работающего" кода. Почему? Потому, что он не учитывает, что рано или поздно, наш endpoint отвалится. Этот пример не учитывает так называемые edge case — пограничные, "плохие случаи". Когда вы начинаете писать код, задумайтесь о том, что может пойти не так. На самом деле я сейчас говорю не только об удаленных вызовах, а обо всех ресурсах, которые находятся вне зоны вашего контроля — пользовательский ввод, файлы, сетевые соединения, даже база данных. Все что может сломаться, сломается в самый неподходящий момент и единственное что вы можете с этим сделать — быть к этому готовым настолько, насколько это возможно.
К сожалению, не все проблемы настолько очевидны. Есть целый ряд проблемных мест, которые почти гарантированно порождают баги. Например, работа с локалью, с часовыми поясами. Это боль и крики "на моей машине все работает". Их просто надо знать и аккуратно с ними работать.
Кстати насчет пользовательского ввода. Есть очень хороший принцип, который говорит о том, что любой пользовательский ввод считается некорректным пока не будет доказано иное. Другими словами — всегда валидируйте то, что ввел пользователь. И да, на сервере тоже.
Итого:
- Сначала заставьте код работать,
- Потом сделайте его хорошим,
- Не забывайте про edge cases и обработку ошибок.
Теперь поговорим о поддержке кода
Поддержка понятие сложное, но я бы включил сюда три составляющие — код должен легко читаться, легко изменяться и быть единообразным.
Кто пишет комментарии на русском? Никто не пишет? Замечательно. В общем одна из проблем — это код на не английском языке. Не надо так. У меня был кусок кода с классами на Норвежском языке, и я просто не мог выговорить их названия. Это было печально. Очевидно, что поддержка такого кода (для не норвежцев) будет не тривиальной задачей. Но такое бывает редко.
Вообще, легкость чтения это про именование и структуру. Имена сущностей — классов, методов, переменных, должны быть простыми, читаемыми и нести смысловую нагрузку. Возьмем наш предыдущий пример.
public string Do(int x)
{
using (WebClient xx = new WebClient())
{
return xx.DownloadString("https://some.super.url");
}
}
Вы можете понять, что делает метод Do не смотря в реализацию? Вряд ли. Аналогично с названиями переменных. Для того, чтобы понять, что за объект xx нужно искать его объявление. Это отнимает наше время, мешает пониманию того, что, в общих чертах происходит в коде. Поэтому имена должны отражать суть действия или значения. Например, если переименовать метод Do в GetUserName код станет немного понятнее и в ряде случаев нам уже не придется смотреть в его реализацию. Аналогично с названиями переменных в виде x и xx. Правда есть общепринятые исключения в виде e для ошибок, i,k — для счетчиков циклов, n — для размерностей и несколько еще.
Опять же, для примера, возьмите свой код, который вы написали месяц назад и попробуйте его бегло прочесть. Вы понимаете, что там происходит? Если да — я вас поздравляю. Если нет, значит у вас проблема с читаемостью кода.
Вообще, есть такая интересная цитата:
«There are only two hard things in Computer Science: cache invalidation and naming things.» © Phil Karlton
Есть только две сложные вещи в Computer Science: инвалидация кеша и именования.
Вспоминайте ее, когда будете давать имена своим сущностям.
Вторая составляющая читабельного кода это его сложность или структура. Я говорю про тех, кто любит писать шесть вложенных ифов, или вписывать колбек в колбек колбека внутри колбека. В JavaScript есть даже такой термин – Callback Hell.
Пожалейте ваших коллег и самих себя. Спустя неделю вам придется буквально продираться сквозь этот код что бы что-то в нем исправить или добавить. Избежать этого не так сложно:
- Пишите короткие функции,
- Избегайте большого количества ветвлений или вложенности,
- Выделяйте логические блоки кода в отдельные функции даже если не собираетесь их повторно использовать,
- Используйте полиморфизм вместо if
Теперь поговорим о еще одной сложной вещи — о том, что хороший код легко менять. Кому знаком термин Big ball of mud? Если кому-то не знаком — посмотрите на картинку.
Каждый модуль зависит от каждого модуля и изменения контракта в одном месте скорее всего приведет к пришествию полярной лисички или, как минимум, к очень длительному дебагу. Теоретически, бороться с этим достаточно просто – уменьшайте зависимость вашего кода от вашего же кода. Чем меньше ваш код знает о любых деталях реализации, тем будет для него лучше. Но на практике это намного сложнее, и ведет к переусложнению кода.
В виде советов я бы сформулировал это так:
- Прячьте ваш код настолько глубоко, насколько это возможно. Представьте, что завтра вам придется вручную его удалять из проекта. Сколько мест вам придется исправить и как сложно будет это сделать? Постарайтесь минимизировать это количество.
- Избегайте циклических зависимостей. Разделяйте код на слои (логика, интерфейс, доступ к данным) и следите что бы слои "нижнего" уровня не зависели от слоев "верхнего" уровня. Например, доступ к данным не должен зависеть от пользовательского интерфейса.
- Группируйте функциональность в модули (проекты, папки) и прячьте классы внутри них оставляя только фасад и интерфейсы.
И рисуйте. Просто нарисуйте на листочке, как ваши данные обрабатываются приложением и какие классы для этого используются. Это поможет вам понять переусложненные места до того, как это все станет непоправимо.
И наконец про единообразие. Всегда старайтесь придерживаться единого стиля, принятого в команде, даже если вам это кажется неправильным. Это касается и форматирования, и подходов к решению задачи. Не надо использовать ~~ для округления, даже если это быстрее. Команда это не оценит. И начиная писать новый код, всегда смотрите в проект, возможно что-то из нужного вам уже реализовано и это можно использовать повторно.
Итого:
- Правильное именование,
- Хорошая структура,
- Единообразие.
Код должен быть достаточно производительным
Давайте немного похоливарим. Следующее требование которые мы рассмотрим — код должен быть достаточно производительным.
Что я имею ввиду под словом "достаточно"? Наверное, все слышали, что преждевременные оптимизации — это зло, они убивают читабельность и усложняют код. Это правда. Но также правда и то, что вы должны знать свой инструмент, и не писать на нем так, чтобы веб клиент почты загружал Сore I7 на 60%. Вы должны знать типичные проблемы, которые приводят к проблемам с производительностью и избегать их еще на этапе написания кода.
Давайте снова вернемся к нашему примеру:
public string GetUserName(int userId)
{
using (WebClient http = new WebClient())
{
return http.DownloadString("https://some.super.url");
}
}
У этого кода есть одна проблема — синхронное выполнение загрузки по сети. Это I/O операция которая, заморозит наш поток до момента своего выполнения. В десктопных приложениях это приведет к повисшему интерфейсу, а в серверных, к бесполезной резервации памяти и исчерпания количества запросов к серверу. Просто зная подобные проблемы, вы уже можете писать более оптимизированный код. И в большинстве случаев этого будет достаточно.
Но иногда, нет. Потому перед тем как писать код, нужно заранее узнать какие требования с точки зрения производительности к нему ставятся.
А теперь поговорим за тесты
Это не менее холиварная тема чем предыдущая, а может даже по более. C тестами все сложно. Начнем с утверждения — я считаю, что код должен быть покрыт разумным количеством тестов.
Зачем нам вообще нужен Code Coverage и тесты? В идеальном мире — не нужны. В идеальном мире код пишут без багов, а требования никогда не меняются. Но мы живем в далеко не идеальном мире, поэтому тесты нам нужны для того, чтобы быть уверенными что код работает правильно (там нет багов) и что код работает правильно после того как что-то поменяли. Это та выгода, которую нам приносят тесты. С другой стороны, даже 100% (в силу специфики расчета метрик) покрыте тестами не гарантирует что вы покрыли абсолютно все. Более того, каждый дополнительный тест еще замедляет разработку так как после изменения фунционала вам придётся еще и обновлять тесты. Поэтому количество тестов должно быть разумным и основная сложность как раз и состоит в том, чтобы найти компромисс между количеством кода и стабильностью системы. Найти эту грань достаточно сложно и универсального рецепта как это сделать нет. Но есть несколько советов, которые вам, возможно помогут это сделать.
- Покрывайте бизнес логику приложения. Бизнес логика это все, для чего приложение создается, и она должна максимально стабильна.
- Покрывайте сложные, высчитываемые вещи. Расчеты, преобразования, сложные мерджи данных. То, где легко ошибиться.
- Покрывайте баги. Баг — это флаг который говорит нам, что тут код был уязвим. И это хорошее место что бы написать здесь тест.
- Покрывайте часто переиспользуемый код. Высока вероятность того, что его будут часто обновлять и нам нужно быть уверенными, что добавив что-то одно мы не сломаем другое.
Не стоит покрывать без большой необходимости
- Чужие библиотеки — ищите библиотеки, код которых уже покрыт тестами.
- Инфраструктуру — DI, автомаппинг (если там нет сложного маппинга) и так далее. Для этого есть e2e или интеграционные тесты.
- Тривиальные вещи — присвоение данных полям, проброс вызовов, и так далее. Почти наверняка вы найдете гораздо более полезные места что бы покрыть их тестами.
Ну вот в общем то и все.
Подведем итоги. Хороший код это -
- Работающий код
- Который легко читать,
- Легко менять,
- Достаточно быстр,
- И покрыт тестами в нужном количестве.
Удачи вам в этом нелегком пути. И скорее всего, your gonna hate this. Ну, а если все же нет, — добро пожаловать.
Комментарии (39)
amphasis
22.11.2018 11:45+3На КДПВ изображена микросервисная архитектура? :)
Zoolander
22.11.2018 17:34смотрите, на картинке изображена архитектура, которая очень сильно зависит от компонента, наиболее приближенного к конечному пользователю
исходя из этого, можно сказать, что это архитектура с нарушением инверсии зависимости
HerrDirektor
22.11.2018 12:07-3Аналогично с названиями переменных в виде x и xx. Правда есть общепринятые исключения в виде e для ошибок, i,k — для счетчиков циклов, n — для размерностей и несколько еще.
Со времен динозавров существует венгерская нотация же. Не смотря на критику, ИМХО, крайне удобный подход, независимо от строгости типизаций используемого языка.
worldmind
22.11.2018 12:44И в каких случаях из хорошего названия непонятен тип и это проблема?
HerrDirektor
22.11.2018 13:00+1Вот есть условная переменная с хорошим названием (хорошее же?) CurrencyType — определите по ее названию, что там? int, string, byte… итп?
dmitry_dvm
22.11.2018 13:05+2decimal
HerrDirektor
22.11.2018 13:32В зависимости от задачи, я могу в ней держать string в виде строкового кода валюты держать. «RUR», «EUR» и т.п.
И об этом вы не догадаетесь до тех пор, пока не увидите логику работы с этой переменной.
worldmind
22.11.2018 14:20+11. Я же не зря уточнил, что это должно быть проблемой, если вы не знаете как в проекте задаются валюты вам, независимо от названий переменных, придётся куда-то посмотреть чтобы увидеть справочник или пример использования в другом месте.
2. В хорошем коде это enumHerrDirektor
22.11.2018 14:53Вот честное слово, не хочу развивать холивар на эту тему.
Давайте останемся при своих — мне нравится венгерская нотация (и ее вариации) еще с прошлого века, и я нахожу ее полезной (для себя) до сих пор (даже в PHP), ну а вам (по вашим личным предпочтениям) — нет.
Drag13 Автор
22.11.2018 13:06C# строго типизирован и венгерская нотация не приносит здесь большой пользы. Всегда можно указать явный тип вместо var. Плюс если тип поменяется компилятор сам подскажет где поправить.
HerrDirektor
22.11.2018 13:09-1Вы рассуждаете как Торвальдс — «компилятор сам знает, что там» :)
Я же рассматриваю ее с точки зрения удобства чтения кода человеком, а не компилятором.Drag13 Автор
22.11.2018 13:20Чтение дело субьективное но мне так больше нравится
decimal totalAmount = GetTotal();
чем
var d_amount = GetTotal();
Впрочем, еще раз, это дело привычки.
HerrDirektor
22.11.2018 13:26Венгерская нотация — это же не постулат какой-то, это относительный пример сокращений для упрощения чтения кода :)
По сути, это тоже венгерская нотация:
Bitmap bmpSrc = new Bitmap (); Image impDest = ... Button btnReset = ...
AndreySitaev
22.11.2018 13:35Уточните, я мог отстать от индустрии: разве префиксы типа d, n, s…, «подсказывающие» тип переменной, не являются deprecated-практикой в C#? Да еще и в snake_case…
Очевидно же, что d_amount не удовлетворяет большинству принятых (включая Microsoft recommended) соглашений о именовании переменных.
Что же до var / …
Если тип очевиден (конструктор), резонно же использовать var:
var man = new Human(Gender.Male); // но Human woman = GodObject.SpawnWoman();
HerrDirektor
22.11.2018 13:48Уточните, я мог отстать от индустрии: разве префиксы типа d, n, s…, «подсказывающие» тип переменной, не являются deprecated-практикой в C#?
Компилятор в логах осуждает, да.
Igor_Shumilov
22.11.2018 13:21+1Сначала решаем проблему, потом делаем код красивым
Тут главное, чтобы это «потом» наступило.HerrDirektor
22.11.2018 13:34+1Лично знаю 2 примера, когда слишком торопливо выброшенный на рынок продукт почил в бозе, а конкуренты через полгода вышли с отлаженной схемой и загребли рынок.
Поэтому грань здесь весьма тонкая.
AndreySitaev
22.11.2018 13:53+1Все написанное выше — IMHO. Интересны сторонние мнения…
Мне кажется, что автору стоило бы сделать пару уточняющих комментариев по собственному примеру.
— 1 —Это замечательный пример «работающего» кода. Почему? Потому, что он не учитывает, что рано или поздно, наш endpoint отвалиться.
Почему бы не предположить, что в проекте выбрана такая схема обработки исключений, что исключение обрабатывается выше по цепочке, а не в самом методе Do(int x)?
Т.е., наверное, следует как-то отметить, что вызвавший код, к примеру, может сам обработать исключение. Что, если junior (а программист на опыте, вероятно, в подобных советах не нуждается), решит непременно лишний раз обработать исключение, следуя совету автора, но «потеряет» при этом call stack, написав код вида:
try {// сделать что-то } catch (Exception e) { // вывести что-то в лог throw e; }
Если автор рассчитывает на не самую подготовленную аудиторию, может, стоит осветить поподробней?
— 2 — Точно так же, возможно, не стоит безапелляционно утверждать, что этот же метод обязан быть асинхронным. Возможно, метод выше по стеку (вызвавший, в конечном итоге, наш метод Do), сам вызван в отдельном потоке / Task. Тогда, вероятно, наша асинхронная версия Do всегда будет вызываться как
string markup = Do(42).Result;
Асинхронность всегда имеет тенденцию «подниматься вверх по стеку». Если в дизайне проекта её нет, то начинать вводить её надо не с метода Do().HerrDirektor
22.11.2018 14:02ТС очевидно слишком упрощает задачи/подходы в рамках «слишком общей» статьи.
Но вы правы, было бы неплохо добавить несколько абзацев с видением решения проблемы со стороны автора.
Drag13 Автор
22.11.2018 14:22Очень дельные замечения. Но это больше об интеграции кода в существующий проект. А здесь мне были нужны простые примеры проблемы/решения. Но уточнить этот момент однозначно стоит, спасибо.
ivorobioff
22.11.2018 13:58У меня лично есть одно правило. Сделать чтобы работало, а потом отшлифовать все методом мелких рефакторингов. Тогда и задача будет выполнена и код не будет выглядеть страшным.
vyatsek
22.11.2018 20:16И тут меня сразу тянет писать какие-то абстракции, какую-то инфраструктуру, чтобы оттянуть момент реальной работы.
Это называется проектирование наперед. Мне вообще непонятно почему большинство разработчиков пытаются в коде воплотить какую-то серебряную пулю, которая будет решать все проблемы на свете.
Почему нельзя написать дубово, просто и понятно. А когда потребуется поменять, то оценка времени будет точной в силу простоты кода.
На практике же напишут generic фреймворк, который по фантазиям разработчика должен решать какие-то выдуманные варианты использования, в нем будет куча защит от дурака и т.д. В результате для того, чтобы сделать простую фичу, надо выкинуть общий код который не используется, все упростить, адаптировать(отрефакторить) текущий код под новую фичу, чтобы в результате весь код смотрелся органично в текущих вариантах использования.
sergey_z777
22.11.2018 20:39Пара ремарок:
"… наш endpoint отвалиться." -> «наш endpoint отвалится.»
… «за тесты» -> «про тесты» или «о тестах»Drag13 Автор
22.11.2018 20:40Спасибо за «ться». А вот «за тесты» смотрите выше.
sergey_z777
23.11.2018 01:35Ваш «намёк на одессу» скорее выглядит использованием жаргонного языка…
Drag13 Автор
23.11.2018 08:12Так одесский говор и состоит из жаргонизмов, так что грань тонка.
sergey_z777
23.11.2018 14:42-1Ваше дело, конечно. Хотите выглядеть неграмотным — пишите «за тесты».
Drag13 Автор
23.11.2018 16:15Вы хотите сделать мне стыдно? Простите, это выйдет. Если бы я мог писать так же «безграмотно» как Бабель, я был бы просто в воссторге.
Давайте закроем тему.
jetcar
23.11.2018 15:08я думаю что многие знают как писать хороший код, но всегда есть такая вешь как бизнес который подгоняет разработку и тут если бизнес логика меняется сильно, нужно иногда большие куски переписать + миграция данных, а это долго и может привести к багам потому делаються хаки, потом когда рефакториться всё то многое можно пофиксить, но и тут бизнес не дремлет, наверно хороший код получается только в чисто технических проектах, где спецификация продумывается тем кто умеет писать и потому логика построения всех компонентов примерно одинакова и не меняется со временем ну и новые фичи это всеголишь расширение функционала который был изначально запланирован, построить же бизнес приложение которое которое легко менять значит писать поддержку совсем динамических данных, и тут приложение из простого моментально превращается в очень сложное
sic
24.11.2018 17:09Хороший код должен быть предпочтительнее плохого для хорошего бизнеса. А вот с последним таки и есть проблема, хотя бы потому, что сейчас хороший бизнес — это быстрый бизнес.
Опять же, для примера, возьмите свой код, который вы написали месяц назад и попробуйте его бегло прочесть. Вы понимаете, что там происходит? Если да — я вас поздравляю. Если нет, значит у вас проблема с читаемостью кода.
Совершенно неадекватный критерий. Например, у меня каким-то образом очень хорошая память на процесс написания кода, так что трудностей с пониманием старого кода никогда не возникало. А потом я несколько раз удивлялся, насколько же этот код идет в разрез со всеми мыслимыми метриками качества вообще.
Покрывайте сложные, высчитываемые вещи. Расчеты, преобразования, сложные мерджи данных. То, где легко ошибиться.
Опять же, практика подсказывает, что если пишешь код в вменяемом состоянии, то ошибиться в сложных вещах на порядки сложнее, чем в каких-нибудь совсем тривиальных.Drag13 Автор
24.11.2018 19:32Например, у меня каким-то образом очень хорошая память на процесс написания кода, так что трудностей с пониманием старого кода никогда не возникало
Я бы сказал, что это ваша индивидуальная особенность. Я забываю достаточно быстро. Коллеги +- так же. Отсюда и совет, который хотя и не подходит всем (например вам) но большинству подойдет.
Опять же, практика подсказывает, что если пишешь код в вменяемом состоянии, то ошибиться в сложных вещах на порядки сложнее, чем в каких-нибудь совсем тривиальных.
Согласен. Но потом приходит рефакторинг и хотелось бы быть наверняка уверенным, что код все еще работает правильно.
unnutz
How to Use You're and Your