Комментарии 167
[вероятно, имеется в виду Hacker News — прим. пер.]
Ничего себе, до чего технологии дошли, гугл переводчик уже примечания делает.
я была джуниором, который мнил себя сеньором
Про джунов-то понятно, а я сейчас работаю в коллективе, где половина программистов (не только джуны) мнят себя крутыми техлидами. Так что при получении бОльшего опыта у некоторых просто растут амбиции.
В былые времена я могла сделать очень жёсткое код-ревью.
Зашёл к ней на github.
Не знаю какое она делала там «жёсткое» код-ревью и какой она там «старший» и супер скилованный разработчик.
Но ей бы хотя бы просто встроенное форматирование кода в среде разработки открыть для себя.
Но это ещё ладно, дальше больше. Открыл первый самый проект:
Не дай бог кому-нибудь такого «наставника».
Архитектура важнее, чем придирки. Хотя какую то строчку можно улучшить, но главные проблемы архитектурного порядка. Лучше сразу сосредоточиться на структуре приложения, а не на отдельных крошечных фрагментах кода.
А потом отдельный конфиг для отдельного конфига, а потом не забыть новым программистам донести что этот конфиг значит, не забыть его описать в документации, не забыть почему он сделан так
Если это имеет смысл — совершенно не зазорно иметь несколько конфигов, например разделить «техническую» часть (креды от бд и т.п.) от констант для бизнес логики. Вместо размазывания их по всему коду.
Комментировать конфиг можно прямо в нем самом, многие форматы это позволяют. Хотя по моему опыту редко приходится писать комментарии в конфиге, обычно и так все понятно.
возможно получить проблемы при переносе системы (например с виндовс машины перетаскиваем в докер контейнер)
Честно говоря, вообще не понял о чем вы. Какие проблемы можно получить, если у вас есть конфиг при переносе в докер контейнер, которые можно было бы избежать без конфига?
веселее может быть только этот конфиг потерять и потом со слезами на глазах восстанавливать.
Что значит «потерять» конфиг? Куда он денется из репозитория?
Не знаю Go, но, как минимум, пересоздание массивов при каждом вызове метода выглядит подозрительно.
Почему вы решили, что массив пересоздают? Не знаю что там у go, а в сях этого массива вообще может не существовать после компиляции.
Так можно оправдать любое неоптимальное решение, но только потом такие решения накапливаются и получается не очень. Современный софт часто тому пример. Если можно написать нормально, то почему бы этого не сделать?
Я не понимаю, с чего вы взяли, что нормально — это с вынесенной константой? В некритичном участке кода читабельность гораздо важнее скорости, поэтому нормально — это так, чтобы связанный логически код не расползался по отдаленным местам программы.
В свете комментария (чуть нижев треде) о том, что это вообще toy-project на пару сотен строк, считаю этот спор вообще лишним, а исходный наброс — глупостью.
Не знаю Go, но, как минимум, пересоздание массивов при каждом вызове метода выглядит подозрительно.
Так это с точки зрения машины подозрительно (возможно, компилятор не соптимизирует). Но код — он ведь не для машины. Код — это литературное произведение, написанное человеком для человека.
Так что все ок с массивами.
* перед фигурной открывающей скобкой есть/нет пробела…
* смущает наличие запятой после последнего элемента
* по-моему название одной из локальных переменных string одновременно является ключевым словом, что
* почему между первыми двумя массивами нет пустой строки, а перед simpleRegion есть?
* смущает наличие запятой после последнего элементамногие языки это допускают без проблем (не добавляют пустой элемент в конце), а некоторые «стили программирования» явно это рекомендуют (чтобы при добавлении элемента в конец получался «дифф по существу» и не приходилось смотреть, только ли запятая добавилась в предыдущей строчке);
* по-моему название одной из локальных переменных string одновременно является ключевым словоммне кажется, это всё-таки имя типа (в Go для меня несколько неожиданный синтаксис).
Я не разбираюсь в языке
Ну, так может для начала разобраться? ;)
* перед фигурной открывающей скобкой есть/нет пробела…
И не должно быть — так форматирует встроенный
go fmt.* смущает наличие запятой после последнего элемента
А она там должна быть — требования синтаксиса Go.
* по-моему название одной из локальных переменных string одновременно является ключевым словом, что
Нет там переменных с названием string. По крайней мере в пределах скриншота.
* почему между первыми двумя массивами нет пустой строки, а перед simpleRegion есть?
Это уже откровенная придирка, но даже тут у меня есть объяснение: вероятно, пробел отделяет декларацию мап (мапы тут по сути играют роль констант/статических данных, хотя они фактически и являются переменными; именно за это, кстати, код выше и критикуют).
По моему, как раз удобно сделано. Берём регион (напрмер Франкфурт), по нему получаем название страны (Германия), а уже по названию страны берём её код. Довольно наглядно.
Плюс, если говорить про дальнейшую поддержку, возможно появится потребность получать код по другим регионам Германии, что будет сделано простым добавлением элемента в regionsToCountries. Если мапить напрямую регион — код, то наглядность исходного текста сильно упадёт.
Возможно, на GO можно как то изящней это сделать, я код на этом языке вижу первый раз в жизни )).
Бывают ситуации когда действительно каждый раз нужен новый массив, компилятор должен пропустить такое.
Судя по всему это ассоциативный массив, а для них структура хранения в памяти весьма сложная и не уверен, что какой-нибудь язык вообще такое оптимизирует.
Хеш-таблица — не такая уж и сложная структура. Во многих языках достаточно написать static при объявлении, чтобы он не создавался каждый раз.
static это про другое, она просто создастся только при первом обращении, но тогда нужно помнить про thread safe.
А я имел в виду compile time вычисления, чтобы в рантайме она вообще в ДКА посчиталась или еще что нибудь прикольное.
нужно помнить про thread safe
В нормальных языка не нужно — оно для каждого треда своё создаётся.
А я имел в виду compile time вычисления
enum вместо static и будет compile time
чтобы в рантайме она вообще в ДКА посчиталась
Конечный автомат предполагает кучу условный переходов. Едва ли это будет быстрее подсчёта хеша.
Эти параметры надо вынести в конфиг и читать 1 раз при старте приложения.
Что будет если кто-то подаст в функцию 2 элемента или абракадабру? Код дальше поедет исполняться, так как нет обработки и проверки. По хорошему в go надо выдавать ошибку, чтобы снаружи знали, что что-то пошло не так. В противном случае потом неделю будешь искать и править «кривые» данные в базах и соседних сервисах.
Результаты тестов штатным бенчмарком:
Её код:
BenchmarkGetRegion-8 5000000 249 ns/opПосле выноса переменных:
BenchmarkGetRegion-8 30000000 39.8 ns/opКод почти в 7 раз производительнее. Да, компилятор go не умеет пока что, оптимизировать такое.
Согласен, хардкодить справочные значения не очень хорошо.
Про попытку передачи неправильного значения, в процитированном вами коде непонятно, так как там часть метода и непонятно есть она дальше или нет (в git нет, это точно).
UPD1: Ещё я могу предположить, что аллокация слайсов внутри методов сделана специально, что бы эти переменные не торчали в области видимости всего пакета. Возможно вместо переноса переменных достаточно объявить их внутри методов как const, но в целом это проблема скупости языка.
UPD2: зашёл в репу, коду 4 года, описание репы «This is a toy project». Весь проект, в котором «менеджер попросит 15 раз в день добавлять по региону» занимает 138 строк.
А если менеджер попросит 15 раз в день добавлять по региону
Значит надо бить его по голове веслом, за организацию управления командой или единственным разработчиком. В мире победившего Electron борьба за оптимизацию ценой ухудшения читаемости выглядит крайне смешно.
книги написаны именно так
1 2, 3 4 5 6 7 8 9 А Б В?
«Ссылочность» в этом случае у нас полностью в голове, а вот для читаемости используются именно копии всего, от букв до слов и тезисов.
1 — да
2 — неужели
3 — вы
4 — правда
5 — представляете
6 — себе
7 — что
8 — книги
9 — написаны
А — вот
Б — именно
В — так
Вот именно что буквы и слова, а не рисунки объектов и прочего. Типа, слово — это пойнтер на класс объекта или действия, который для всех (примерно) един :-)
Тезисы тоже будут подкрепляться ссылками на источники, если они не широко общеизвестны.
И лично для меня магические константы и структуры данных внутри кода — дикость. Я не делаю так даже в самых «наколеночных» bash-скриптах или одноразовых программках. Потому что говнокод начинает замедлять тебя (не только написание и изменение, но и чтение) уже в течение часа, а именно в таких вот «наколеночных» программках часто важна скорость изменения и гибкость и поэтому там важно писать чистый код, как ни странно это может кому-то показаться.
Ради попытки экономии на спичках вы создаёте зависимость от глобального конфига (что само по себе мешает тестам) или отдельный конфиг, который планируете инициализировать каждый раз (даже когда функция не вызывается). Вы не сэкономите 200 наносекунд, а потеряете.
Можно сделать lazy-конфиг, но это не тривиально.
Потенциальные преимущества перекрываются такими же потенциальными недостатками.
В случае, если изменения маппинга происходят раз в год, константа была бы удобнее, чем внешний конфиг.
Никогда этого аргумента про тестирование при изменении конфига не понимал. Если вынести все в конфиг — то он становится кодом. И изменения в нем точно так же может сломать все то же самое, что эквивалентный код. Так почему тестирования меньше?
Нет. Принято решение, что изменение конфига можно не ревьювить и не тестировать потому что "тут уже все протестировано". Почему того же решения нельзя сделать про изменение этих же двух переменных в коде?
Или, в обратную сторону. Если изменение двух переменных в коде признается достаточным, чтобы все перетестировать, то почему изменение тех же двух переменных в конфиге таким не признается?
Но тогда позиция "все что может быть вынесено в конфиг должно быть вынесено в конфиг" нуждается в уточнении в смысле того, что именно может.
А то ведь можно дойти до того, что половина логики будет лежать в "конфигах" на самописном скриптовом языке.
Как раз потому, что код мы через тесты и прочую бюрократию прогоняем и это долго. А конфиги изменять быстро. Поэтому в какой-то момент в язык конфигурирования добавляется возможность быстро поправить логику.
Чтобы переконфигурить апп нужно поменять значение переменной в конфиге и перезапустить апп, оно уже было протестировано на первом этапе.
А потом бац! И ваше приложение падает, т.к. в одну мапу вхождение добавили, а в другую — нет.
все что может быть вынесено в конфиг должно быть вынесено в конфиг, однозначно.
Точно ли это истинное утверждение? Проверьте его на количестве секунд в минуте. Это может быть вынесено в конфиг.
При этом в константу вынести можно, в конфиг — бессмысленно.
Код почти в 7 раз производительнее.
Так, может, вообще на ассемблере всё переписать? Знаете, во сколько раз производительней будет?
Вынос в константы однозначно нужен. А вот вынос в конфиг может оказаться и оверинженерингом. Про YAGNI тоже не нужно забывать.
Вас не смущает, что добавление нового региона, страны или маппинга между ними потребует входа в код, изменения захордкоженных параметров и снова деплой на сервер?
А вас не смущает, что реализация подобной подсистемы конфигов — это отдельная задача, которая должна быть отдельно согласована с тем самым менеджером? И что потом этот код с конфигами придется тестировать и поддерживать?
Вот есть такое мнение, что производительность программистов может отличаться вплоть до десятичных порядков. Мне кажется, вот где-то в этой области отличие и кроется — один за полчаса написал ф-ю, простую, которую можно легко читать и исправлять потом, а другой потратил неделю на реализацию конфигов. Еще и фабриками синглтонов обмазать надо до кучи все это дело.
Скажем, преждевременная нормализация {«DE», «fra», \U0001F1E9} -> {«DE», «fra»}, {«DE», \U0001F1E9} — это очередной подводный камень при попытке обновления этого кода. Тут и так опечататься раз плюнуть.
А о том, что вообще такое это \U0001F1E9, я и задумываться лишний раз не хочу, это трата времени. Впрочем, сеньорам виднее.
fmt.Sprintf("%s — %s %s", name, ip, region) тут типа printf
а тут прибавление
url = «cloud.digitalocean.com/droplets» +
strconv.Itoa(droplet.ID)
а вот опять плюсы
systray.SetTooltip(«You have » + strconv.Itoa(len(dropletList)) + " droplets")
вспоминаю очень строгого препода, который говорил что все константы, строковые в том числе, должны быть извлечены наверх проги
мож что то ещё тут, подозреваю что слишком специализированный код но я не знаю это уго
Какой способ быстрее решит задачу:
1) Дернуть сержанта, объяснить задачу и сказать, чтобы он выводил бойцов на работу.
2) Вывести бойцов, объяснить бойцам, что от них требуется, произнести мотивирующую речь и отправить на работу.
3) Вывести бойцов, отдать приказ приступать к работе. Когда в толпе появится возмущение, ударить самому борзому по морде и риторически спросить «Кто еще против советской власти?»
Больше того: бывает, что тот код, который один сеньор считает эталоном качества, у другого не пройдёт код-ревью с резолюцией «переписать полностью».
Один из них как минимум делает что-то не так, а возможно даже оба.
Вместо того чтобы спрашивать «где хорошо», спросите лучше «где плохо и как это мешает?».
Метрики вещь сложная и неоднозначная, но имеют место быть в дополнение.
Все очень просто.
1) Не выделяй память, если можно не выделять
2) Знай какие операции блокирующие, а какие нет. Очевидно читать файл или слать запросы в сеть в гуёвом потоке дело неправильное.
3) Не городи абстракций в одноразовых решениях или на среднем уровне архитектуры приложения. Конкретный код, под конкретное бизнес решение, никогда не будет переиспользован.
4) Соблюдай стайл-гайд
5) Если пишешь костыль, оставь огромный комментарий с цепочкой размышлений, который привёл к появлению костыля.
6) Если условие сложное, разбей на несколько булевых переменных с нормальным названием и скомбинируй. Если невозможно (например сложная регулярка), то оставь понятный текстовый пример, когда условие срабатывает и когда не срабатывает.
7) Не используй редких английских слов в названиях
8) Погугли хоть раз в жизни " best practices".
9) Не пиши сложно и оптимально, если код вызывается раз в час/сутки/жизнь.
10) Даже если ты невероятно крут и умен, помни, что читать и поддерживать твой код будут люди менее компетентные. Это не значит, что надо писать хуже, но значит, что в каждой точке, над которой пришлось подумать и сделать неочевидно — нужно оставить комментарий.
Качество кода обычно вещь всё же более сложная и обширная.
Ну соблюдение единого стиля и именование переменных это уж совсем базовые вещи которые должны быть очевидны
Ох, если бы… :D Я проревьювил сотни тысяч строк кода :) Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных. Причем, чем более опытный разработчик, особенно если опытнее тебя, тем больше получишь в ответ надменных ответов в стиле "я так всегда писал, мне так удобнее".
Конечно, я сильно утрирую, но факт остается фактом — в 99% ревью, которые я смотрел нарушается по крайней мере 2 правила из этих 10 :)
Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных.
Потому что форму оценивать проще, чем содержание. И каждый ревьювер считает, что сделает очень важное дело, если не допустит в репозиторий код, где ничего не значащий пробел поставлен не с той стороны.
То, что содержание важнее формы совсем не отменяет важность формы, если мы говорим о качественном коде. Код выполняющий свою задачу — это рабочий код. Конечно, он лучше не рабочего. Но если его невозможно читать и если в нем "ничего не значащие пробелы" вообще не вписываются в структуру кода во всём проекте — этот код НЕ качественный. Даже если он супероптимизирован (возможно без причины), но понять его можно только после недели разглядывания и сделать это могут полтора человека в компании — это НЕ качественный код.
Ревью, к слову, не призвано к улучшению работоспособности кода. Предполагается, что код присланный на ревью УЖЕ работает. Ревью как раз призвано улучшить качество и как побочный эффект — увидеть пару багов :)
Тому много причин. Я не знаю, работали ли вы над проектом, код в котором писали 30-50 разных людей и в разное время, но если работали, то удивлен, как вы умудрились не заметить как быстро, буквально за полгода, код превращается в мешанину из неправильных скобочек, комбинации табов/пробелов, разных стилей наименований и абсолютно непонятных костылей.
Любые, даже очень дебильные правила, лучше их отсутствия. Потому что я видел проекты в которых не нашлось человека, который бы "тратил время людей в пустую". И это был ад при первом столкновении с таким кодом.
Я не говорю, что фанатичность в этих вопросах — это самое главное. Просто форма, в вопросах качественного кода имеет не малый вес.
В целом, весь этот спор похож на спор о важности секса в отношениях :) Да, секс — не главное, но без секса хорошие отношения с партнером для меня не существуют (субъективно).
Так же и соблюдением стайл-гайда. Рабочий код можно плодить и без него, но качественный — никогда.
Примерно так по вашему выглядит офис без дресс-кода?
Был дан очень грамотный комментарий. Если на ревью критично соблюдение стайлгайдов, сделайте жизнь разработчикам проще — добавьте гитовые пре-коммит хуки, которые будут проверять и исправлять стайлгайдовые ошибки. Вот и все решение. И себе время сэкономите и коллегам своим.
ИМХО.
- Комментарии в коде часто недостаток, чем польза. Чаще всего они содержат
//todo, либо//fixme, либо// это переменная. Если требуется комментарий, то надо подумать, а нельзя ли было написать лучше. Чаще всего, код документирует сам себя. Выбирай грамотные названия переменных. Комментарий ставится только тогда, когда без него никуда. - Даже если код вызывается редко это не отменяет необходимости сделать его оптимально.
- Абстракции на уровне бизнес логики не менее важны, чем на уровне поддерживающего слоя.
UPD: Комментарии нужны для того, чтобы описать почему здесь сделано именно так, а не иначе, а не для ответа на вопрос что здесь происходит.
Написание таких хуков займет прилично времени, но в целом да, это выход. Я пишу на плюсах и даже самые лучшие IDE не умеют форматировать списки шаблонов и переменных правильно и нигде нет достаточно умных правил.
Даже если разработать простой стайл гайд с простыми правилами, невозможно автоматизировать разрабов называть переменные содержательно и нормально описывать условия и заставлять оставлять атрибуты к функциям/переменным или проставлять const / noexcept у методов/переменных.
Чаще всего, код документирует сам себя.
Я такого не видел за всю карьеру. Тем более на С++ :) Тем более на старом С++. Однако при соблюдении всех гайдов, особенно по части названий, к этому можно приблизиться.
Комментарии в коде часто недостаток, чем польза
Хреновые коментарии, написанные не по инструкции — да. Правильные комментарии, которые я описал в п.5, которые отвечают на то, зачем и почему этот код тут — еще ни разу не портили код.
Даже если код вызывается редко это не отменяет необходимости сделать его оптимально
Зависит от того, хотим мы в итоге выпустить продукт или нет. Если не хотим, можем писать оптимально хоть всю жизнь :)
Абстракции на уровне бизнес логики не менее важны, чем на уровне поддерживающего слоя.
Есть грань, за которую не нужно заходить. Все крупные компании, которые я видел, тратят кучу времени на обслуживание таких абстракций, в то время как переписать и протестировать многое было бы намного быстрее. Или хотя бы поддерживать 4 локальных решения было бы дешевле, чем 1 универсальное. Но это всегда тема для обсуждения в конкретных условиях и примерах, не имеющая смысла в отрыве от задачи.
Помимо отступов и скобочек есть еще миллион соглашений о наименованиях переменных и классов, расставлении атрибутов, порядок передачи аргументов, максимальное количество аргументов, правила использования макросов, правила описания шаблонов, правила обработки ошибок и т.д. и т.п.
Десятки правил неавтоматируемы, но совершенно легко понимаемы человеком.
Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных.
Стилевые проблемы обычно решаются добавлением линтера, которые есть для большинства языков, и хуков на коммит/мердж.
А названия переменных — вообще однa из сложнейших задач в программировании :)
[UPD] не обновил комментарии…
/// @tparam SessionT Описание контрактов для типов
/// @tparam MessageT
template <
typename SessionT,
typename MessageT
> //Я не смог заставить ни одну IDE выровнять эту скобочку правильно
class Connector final //Нет виртуального деструктора -> final обязателен
: public BaseConnector <
SessionT,
MessageT,
Connector <
SessionT,
MessageT
>
>
{
using BaseT = BaseConnector <
SessionT,
MessageT,
Connector <
SessionT,
MessageT
>
>;
public:
//Если метод - обработка события, то он должен начинаться
//с on_ и потом имя события с Большой буквы
void on_Connected();
void on_Finished();
//Но обычные методы должны начинаться с маленькой и без подчеркиваний
//Проверить что const и noexcept проставлены, если это возможно
const Value& getValue() const noexcept;
void setValue(const Value& value) noexcept;
//Если класс final, то нужно проверито отсутствие protected полей
private:
///Если параметров 2, то пишем их в одну линию
void fewParams(int param1, int param2);
//Если параметров больше 3-х, то оформляем в столбик
void manyParam(
int param1,
int param2,
int param3,
int param4
);
private:
Value m_value; //не забыли префикс m_ у меременной
};
Я предлагаю не спорить о самом стайл-гайде, о том, что именование методов обработчиков событий одно, а для обычных методов другое, о том что префикс m_, нынче ставить не модно и вот это всё.
Это реальный стайл гайд, реального проекта. Не я его придумал, но с ним уже написано дофигища кода и в целом, он не так уж и плох.
Стоит упомянуть лишь, что здесь еще нет много чего. Нет примера правил по использованию слова auto, нет отражены проблемы с любовью некоторых программистов писать не
int fun(const Value& value);а
int fun(Value const& value);что синтаксически одно и то же, но стайл-гайдом закреплен первый вариант.
Просто скиньте мне ссылку на инструмент, а лучше правила для clang-format, которые мне будут всё это проверять автоматически при коммите.
Все очень просто.
1) Не выделяй память, если можно не выделять
Вы написали «просто», а потом сразу же влепили пункт, который мягко говоря не прост. Экономия по памяти достигается или расходом процессорных циклов, или (когда до этого еще не дошло) усложнением кода, когда можно бы и не усложнять. Даже очевидная свалка в памяти, в духе «храним одно и то же несколько раз в виде копий» — далеко не всегда однозначное зло, если избавление от этой свалки увеличивает сложность и связность кода, а объёмы даже в худших случаях таковы, что волноваться по этому поводу не стоит.
Вопрос о том, сколько и где выделять памяти — он, мягко говоря, очень непростой.
Я не говорю, что задача выделения памяти простая. Там настолько дофига нюансов, что можно книгу и не одну написать.
Я говорю о критериях качественного кода. Программист, знающий нюансы выделения памяти — напишет более качественный код, нежели программист их не знающий и выделяющий каждый байт в куче. Знание этих нюансов — это то, что мы называем опытом и то, что отличает код джуна, от кода миддла и сеньора.
Обычно код пишут так, словно у них дома склад оружия и маленькая армия вокруг :)
Хорошие комментарии могут содержать много очень полезной информации:
— почему сделано так, а не иначе
— какие допущения были приняты при написании этого кода
— какие изменения в будущем предполагаются автором
— какие детали на момент написания коды не были достаточно уточнены
— и т.д. и т.п.
Выше человек написал
Комментарии нужны для того, чтобы описать почему здесь сделано именно так, а не иначе, а не для ответа на вопрос что здесь происходит.
но это несколько упрощённая точка зрения. Комментарии объясняющие «что здесь происходит» (то есть, по сути, дублирующие код) не только бесполезны, но и вредны так как являются той самой очень быстро устаревающей документацией которая, к тому же, запросто может оказаться изначально неверной (когда автор написал в комментарии одно, а в коде, по ошибке или неопытности, реализовал, по сути, не совсем то, что написано).
Но может быть и наоборот. Когда комментарий правильный, а код — нет. Так что различие комментария и кода — это как различие кода с тестом. Если не сходится, то нужно разобраться и что-то из двух поправить. А если комментария нет, то ошибка в логике происходящего никогда не найдется.
В результате наличие комментария, так же как и наличие теста, должно повышать качество кода. Только комментарий используется немного на другом этапе жизни продукта.
Но может быть и наоборот. Когда комментарий правильный, а код — нет. Так что различие комментария и кода — это как различие кода с тестом. Если не сходится, то нужно разобраться и что-то из двух поправить. А если комментария нет, то ошибка в логике происходящего никогда не найдется.
Это не «наоборот». Это ровно тот сценарий, который я описал. В целом, конечно, написание комментариев не меньшее искусство чем написание кода. А может быть и большее (по моим наблюдениям писать более-менее работающий код умеет в разы больше людей, чем разумные комментарии)
Что именно вы вкладываете в это? Я вот, например, пишу парсер CMake-файлов по грамматике на сайте. Там точно нужны комментарии?
Мне видится ровно два вариант:
1) Код настолько тривиальный, что комментарии в нём действительно не нужны (менее вероятно)
2) Комментарии, таки, нужны.
У меня были случаи, когда подобные вещи описывались в виде отдельного TeX-документа, с формулами и их выводами. Но это ИМХО тоже нужно не везде.
Или мы, опять же, говорим о разных допущениях.
Допущения — это те исходные позиции, которые явным образом не зафиксированы в коде, но были обговорены или додуманы при его написании. Мне казалось это довольно очевидно.
А это зачем писать?
Ну, в моём мире редко когда приложение пишется сразу в полном и идеальном виде. Обычно всё начинается с PoC, а дальше развивается. Возможно в вашем — по-другому. Но, опять же, сильно сомневаюсь.
Ну, да, TODO для этого, и заодно описание, почему какие-то костыли были сделаны, возможно, с ссылкой на багрепорт — да, бывает. Но таких комментариев как раз чем меньше, тем лучше.
TODO — это вообще не про это. Оно скорее имеет отношение к предыдущему пункту. Про ссылки на баг-репорты тут уже кто-то (явно имеющий хороший опыт) рядом написал (про смену баг-трекера и тому подобные вещи).
Если у меня есть клиент для некоего внешнего API, и API требует отсылать не больше 2 запросов в секунду, то передать параметр 500ms куда-то там в менеджер очереди запросов — достаточная фиксация?
Нет, недостаточная. Почему именно 500, а не, скажем, 600? А вот комментарий вида «API требует отсылать не больше 2 запросов в секунду» — достаточная.
есть три вида строк (unquoted, quoted, raw), и я выбираю не представлять информацию о том, какого именно вида была строка, в AST, а сразу парсить их в универсальное представление — это надо фиксировать как-то?
Разумеется. Иначе ваш последователь будет ломать голову: «На кой чёрт этот умник стал парсить именно в универсальное представление — это же требует в три раза больше памяти и совершенно не нужно в данном случае?!»
дальнейшие планы формализуются в виде списка фич где-то в JIRA, багтрекере TODO.md или на доске маркером, а не в виде комментариев в коде.
Ага. Потом с доски всё стирают, с JIRA переходят на Redmine (попутно теряя кучу данных) и все такие: «Так, мы же, вроде, хотели что-то там улучшить, но что?! Кто-нибудь может вспомнить?»
часть базы модифицируется вне транзакции
Так, на всякий случай — работа с БД всегда идёт в транзакциях. Если вы не управляете ими явно это не значит что их нет…
А что делать-то, в конце концов? Копировать описание бага в комментарии? А зачем?
А нет никакого универсального рецепта. Это элемент искусства, в некотором роде.
Но код должен быть документирован. Самодокументрованный код подобен атомной бомбе: как атомная бомба всегда падает в свой эпицентр, так и самодокументрованный код всегда соответствует своей документации, какие бы опечатки не содержал. А кто умеет писать без опечаток? Особенно, копипастить.
Поэтому, документация к коду нужна. Можно держать её в отдельном текстовом файле, можно – прямо в тексте программы. Я совмещаю оба подхода. Пишу алгоритм в текстовом файле, после тщательной проверки копирую текст в Visual Studio, превращаю его в комментарии, а уже под комментариями расписываю код программы.
Да, с годами код изменяется. Комментарии около кода всегда правлю, чтобы код им соответствовал, а вот исходный текстовый файл описания часто остаётся без изменений, увы.
Благодаря этому подходу одна из моих разработок развивается (и продаётся) уже тридцать лет. Поменялось несколько компиляторов и ОС, только разрядность ОС менялась дважды. Если бы я не комментировал алгоритмы, то не смог бы так долго поддерживать программу в рабочем состоянии.
Конечно же, к программам — однодневкам всё вышесказанное не относится.
9) Не пиши сложно и оптимально, если код вызывается раз в час/сутки/жизнь.
Мы вот тоже так думали как-то раз. Совсем недавно переписывали и оптимизировали крупный кусок, потому что хотя оно и вызывалось раз в час, но и работало при этом часа полтора, в среднем. (И даже не так уж и плохо написано было, просто работы действительно делается очень много).
- Есть код, который должен выпоняться как можно быстрее.
- Есть код, который должен занимать минимум памяти.
- Есть код, который должен легко читаться.
- Есть код, который должен легко читаться новичком/учеником.
- Есть код, который должен легко читаться человеком без знания английского.
- Есть код, который должен легко отлаживаться.
- Есть код, который должен легко модифицироваться.
- Есть код, который должен просто ожидаемо отработать один раз, и тогда все остальные пункты не имеют значения, а единственным критерием качества является отсутствие багов.
согласен.
Но при работе в команде требуется все-таки пункт с хорошей читабельностью — а значит, приходится придерживаться каких-нибудь правил. Да и если даже пишешь в одиночку — удобнее возвращаться к старому коду, когда он написан так, что ты и через год его легко узнаешь и понимаешь
Но при работе в команде требуется все-таки пункт с хорошей читабельностью
Но не абстрактной читаемостью — а читаемостью для специалистов имеющегося уровня.
Почему я считаю понятной запись
if(~x.indexOf(y))?
Потому что её считают понятной все, кому придётся читать мой код.
Да, всем понятно, но никому не нравилось. Сравнение с -1 читается быстрее и проще.
Есть код, который должен легко читаться человеком без знания английского.
Можете привести пример, когда это необходимо?
Мне довелось пересечься с одним разработчиком, совсем не владевшим английским языком и он был настолько плох, что не смог преодолеть испытательный срок. Вряд ли есть смысл ориентироваться на подобных разработчиков.
— Обучение детей (школьный возраст).
— Встречал код китайской команды, у которых даже в названии одной переменной была шутка на китайском про псов-неудачников. Геймдев, продукт с миллионными заработками (AutoChess).
Но в целом, да, встречается не часто.
- Есть код, который НЕ ДОЛЖЕН легко читаться человеком, без знания национального языка команды разработчиков
ни кто не может объяснить критерии качественности
Это не проблема кода. Что такое "качественная книга" тоже никто объяснить не может. Это фундаментальное свойство качества.
Я запускала ssh на сервере и делала git pull. Уверен, что никогда не видела ни одного пулл-реквеста.
Во-первых, — перебил его Филипп Филиппович, — вы мужчина или женщина?
Например, почему здесь mutex, а не InterlockedExchange, и тому подобное.
Как можно заблаговременно ответить на вопрос "почему именно так?", если даже не представляешь "а как иначе-то?".
кому-то покажется странным и someArray.map->filter-> вместо цикла
а кому-то — цикл вместо цепочных функциональных вызовов
Имеется в виду, в разных командах — разные ситуации, когда может возникнуть вопрос «зачем». Есть вещи, про которые вообще никто не спрашивает. А теоретически можно спрашивать про все. Почему не использовали полифилл? Почему использовали полифилл? В команде надо принять свод правил — и придерживаться его (ну и плюс линтер, конечно, он лишним не бывает)
Он же мог бы и за пробелами и табуляцией в коде следить, создавая скрипты форматирования.
И это действительно работает.
А еще — он станет центром обмена опытом:
подскажет как удобнее настроить среду разработки, и какие вспомогательные скрипты облегчат работу,
подскажет новому сотруднику — с чего начать знакомиться с проектом, где точки входа. Подскажет руководству о проблемах, на которые жалуются работники (где нужно закупить ПО/обновить оборудование/внедрить какую-то технологию/ликвидировать организационно-бюрократический барьер, мешающий работе).
Общаясь со всеми, он видит всех, понимает их настроения, знает их возможности и реальную загрузку.
Поэтому, сможет вовремя заметить профессиональный конфликт в коллективе, например, когда разработчик категорически несогласен с архитектором, и вынужден делать реализацию архитектуры, с которой категорически не согласен — на «отвяжитесь».
В этом случае, «ТехПис» подскажет руководству повнимательнее рассмотреть этот проблемный технический вопрос, с привлечением мнений различных специалистов.
Часто бывает, что люди обладают более широкими знаниями, чем нужно в их текущих задачах.
«ТехПис» поможет свести между собой специалистов для обмена опытом. Подскажет кто что может знать, с кем можно посоветоваться.
Чтобы стать таким человеком, нужно уметь не только грамотно и понятно выражать мысли (что само по себе — уже отдельный ценный навык, который развит не у каждого программиста), но и разбираться в программировании и технологиях, а так же — в психологии.
В одной из организаций, где я работал ранее, подобную деятельность по своей инициативе вела одна из сотрудниц отдела кадров.
Документацию она не писала, а в остальном — все работало именно так.
И дополнительные совещания по проекту назначались при несогласии разработчиков с решением архитектора, и оборудование закупали по жалобам на тормоза компов, и рассматривали вопросы внедрения систем командной разработки, которые знали и рекомендовали сотрудники, которых она опрашивала.
Положительный результат действительно был от этих регулярных разговоров с каждым.
Другое дело — программистов там было не очень много (<20), а когда их сотни, то непонятно как масштабировать этот подход.
подскажет как удобнее настроить среду разработки, и какие вспомогательные скрипты облегчат работу,
подскажет новому сотруднику — с чего начать знакомиться с проектом, где точки входа. Подскажет руководству о проблемах, на которые жалуются работники (где нужно закупить ПО/обновить оборудование/внедрить какую-то технологию/ликвидировать организационно-бюрократический барьер, мешающий работе).
Общаясь со всеми, он видит всех, понимает их настроения, знает их возможности и реальную загрузку.
Поэтому, сможет вовремя заметить профессиональный конфликт в коллективе, например, когда разработчик категорически несогласен с архитектором, и вынужден делать реализацию архитектуры, с которой категорически не согласен — на «отвяжитесь».… подскажет руководству повнимательнее рассмотреть… проблемный технический вопрос, с привлечением мнений различных специалистов.… поможет свести между собой специалистов для обмена опытом.… Чтобы стать таким человеком, нужно уметь не только грамотно и понятно выражать мысли (что само по себе — уже отдельный ценный навык, который развит не у каждого программиста), но и разбираться в программировании и технологиях, а так же — в психологии
По описанию похоже на скрам-мастера…
Я не представляю ситуации, когда после двух часов написания сложнейшего костыля (именно о таких и речь в п.5), у тебя не найдется еще 3-10 минут на написание комментария к нему. Ничем, кроме как ленью оправдать это не получается. К легким костылям — комментарий обычно тупой: "здесь ставлю флаг, потому что без него падает проверка в функции Х".
Видал и такое. Только потом оказалось, что компания переехала с TFS на JIRA и ссылки уже года 4 как не валидны :) Ровно как и ссылки на вики.
К тому же, даже если ссылка валидна — там описана обычно только проблема, а не её решение. В решении обычно всем лень писать что-то осмысленное, поэтому там написано "пофикшено в версии х.у.з".
А даже если и написано что-то, не факт, что под рукой есть доступ к корпоративной сети (например, я могу быть в поездке).
Или еще банальнее, замечал абсолютно за каждым разрабом и даже за собой — тупо лень лезть куда-то по ссылке. Не знаю с чем связано такое пренебрежительное отношение к хождению по ссылкам, но это общая черта.
P.S.
Кто и когда git blame расскажет, нет нужды особо это писать. Да и вообще дело бессмысленное. Скорее всего, если ты забрался так глубоко — человек уже 100 лет назад уволился или просто забыл, что вообще писал этот кусок.
Понятно, что это все такой лигаси, что джуников туда не пускают. и пусть даже автор уволился, но можно будет найти потом через 10-20 лет (да, пришлось как-то править код из 1994 года) и локализировать его правки в то время (а будущим саппортерам — мои нынешние правки).
Неужто в тикетах там настолько подробно всё описывалось, что ими можно даже было заменить комментарии? Всё же тикет это обычно сущность более высокого уровня, чем построчный комментарий с разъяснением, почему решение именно тут не оптимальное или выглядит необычно.
Если нет — это может быть даже хуже чем нормальный комментарий, хоть и на целую страницу, потому что "КТО" — это bus factor в чистом виде, он может уже уволился, или банально делал это всё в кофейном бреду и уже ничего не помнит.
На практие жена — джуниор-вайтишник, 30 лет. Последовательно наступает на одни и те же грабли, что и я много лет назад, и мое присутствие и постоянные код-ревью мало что меняют. Каждый должен пройти свой путь :)
Так что представьте моё удивление, когда в первый рабочий день в стартапе я не нашла никаких тестов. Никаких тестов в интерфейсе. Никаких тестов в бэкенде. Вообще никаких тестов.Нахрена в стартапе тратить время на написание тестов, если на следующей неделе фаундер с просветленными глазами прибежит с «пивотом» и весь код проекта следующее 3 недели придется заново переписывать.
На очень ранних стадиях отказ от всего что не является абсолютно необходимым для достижения результата (тесты, документация, навороченный таск менеджер) является прагматичным решением. Тесты нужно начинать внедрять тогда, когда овчинка стоит выделки: команда разростается и начинает заниматься в том числе поддержкой кода, а не только постоянным дописываением и переписыванием с нуля, требования к системе устаканиваются, возникает архитектура, а цена ошибок выростает.
В усреднённом случае, любая команда начинает заниматься поддержкой кода сразу же после первого commit'а, а не когда-то в абстрактном будущем когда она "разрастается".
Если переписывание кода с нуля не сопровождается переписыванием с нуля требований к системе, то тесты — это наоборот спасительная соломинка, которая поможет хотя бы убедиться, что часть предыдущего функционала по-прежнему работает. Или узнать, что что-то поломалось — причём, узнать быстро, а не когда придут обиженные клиенты (ведь не нанимать тестировщиков в стартап — это тоже прагматичное решение, правда?).
А если сопровождается — то такова жизнь, тесты тоже код, и их придётся выкинуть в ту же самую яму. Но и тут — выкидывают ведь не всё, что-то остаётся нетронутым, а кто будет проверять работоспособность оставшихся фич? Программист за $300/час, или какая-нибудь автоматическая билд-система за $10/месяц?
Не могут же они до посинения тестировать всё снова и снова
Вот вам смешно, а у нас в среднем японском айти всё так и работает, поменял код — бери лист в экселе, проходи всё и заполняй :/
Не могут же они до посинения тестировать всё снова и снова
Ну вообще могут, но не совсем так, как вы описали — у них делаются раунды тестирования, сначала новые фичи, потом — регрессионное тестирование по старым, что сломало добавление нового кода. Если они не уверены, кто и чего трогал, они могут спросить об этом разработчиков и соответственно спланировать регрессию.
А вот так как вы описали тестировать — это работа билд-системы. Перед коммитом быстрые юниты на машине разработчика, и на удалённой машине, по триггеру с накоплением, — интеграционные тесты почти в боевой обстановке, включая тесты безголовым браузером. У нас примерно так и ведутся эти дела, и именно поэтому я могу делать глубокий рефакторинг сравнительно спокойно.
Так и я говорил какую роль могут занимать тестировщики, независимо от того, нашли разработчики хорошее оправдание не писать автоматических тестов или не нашли.
У них работа, знаете ли такая — тестировать всё снова и снова, до посинения, да. И чем меньше покрыто автоматическими тестами — тем сильнее загружены тестировщики. Обратное, впрочем, как правило, неверно.
Переводчику для справки.
"Nada" из оригинала != "Нда".
Это "нда" здесь сильно выбивается из остального ряда (см. "Ничего. Ноль. NaN. Тесты отсутствуют как явление").
Nada = nothing. Перекочевало из испанского в американский английский.
Семь «абсолютных истин» джуниора, от которых пришлось отучиваться