IT systems testing
Project management
IT career
Comments 167
+45
[вероятно, имеется в виду Hacker News — прим. пер.]

Ничего себе, до чего технологии дошли, гугл переводчик уже примечания делает.
+1
я была джуниором, который мнил себя сеньором

Про джунов-то понятно, а я сейчас работаю в коллективе, где половина программистов (не только джуны) мнят себя крутыми техлидами. Так что при получении бОльшего опыта у некоторых просто растут амбиции.
+2
Я, как и автор, мнил себя крутым сеньором в 23 года, и когда гляжу на сегодняшних 23-летних техлидов и 22-летних деливери менеджеров, просто слезы умиления наворачиваются у обычного рядового девелопера :) А оглянешься вокруг — на три буквы некого послать, от должностей в глазах рябит, создается впечатление что девелопер на конторе из 170 человек — я один
+2
В былые времена я могла сделать очень жёсткое код-ревью.

Зашёл к ней на github.

Не знаю какое она делала там «жёсткое» код-ревью и какой она там «старший» и супер скилованный разработчик.

Но ей бы хотя бы просто встроенное форматирование кода в среде разработки открыть для себя.

Но это ещё ладно, дальше больше. Открыл первый самый проект:

image

Не дай бог кому-нибудь такого «наставника».
+1
Архитектура важнее, чем придирки. Хотя какую то строчку можно улучшить, но главные проблемы архитектурного порядка. Лучше сразу сосредоточиться на структуре приложения, а не на отдельных крошечных фрагментах кода.
+23
А можно для людей, не разбирающихся в этом языке, прокомментировать что именно тут ужасно?
+7
Не знаю Go, но, как минимум, пересоздание массивов при каждом вызове метода выглядит подозрительно.
+8
Я иногда создаю массивы в пределах видимости функции. Это удобно особенно если код не конечный и надо работать с данными определенной структуры, чем дописывать лишнюю ветку в case/switch или еще один if… удобнее создать массив и бегать по нему циклом и по мере надобности дописывать и убирать что то. Потенциальный шанс накосячить при изменении кода — меньше, т.к. в алгоритм работы меньше вмешиваются.
0
Мне кажется в данном случае правильнее было бы сделать отдельный конфиг для регионов и иконок к ним. Совсем незачем их прямо в функции инициализировать каждый раз.
0
А потом отдельный конфиг для отдельного конфига, а потом не забыть новым программистам донести что этот конфиг значит, не забыть его описать в документации, не забыть почему он сделан так, а не иначе, возможно получить проблемы при переносе системы (например с виндовс машины перетаскиваем в докер контейнер) ну вот (нецензурный аналог «зачем») вам эти мины замедленного действия? веселее может быть только этот конфиг потерять и потом со слезами на глазах восстанавливать. Ценность специалиста в умении соблюсти баланс, а бездумно делать по паттернам «патамушта так принято».
+1
Конфиг — это не обязательно отдельный файл. Можно, например просто вынести в константы вне скоупа функции.

А потом отдельный конфиг для отдельного конфига, а потом не забыть новым программистам донести что этот конфиг значит, не забыть его описать в документации, не забыть почему он сделан так

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

возможно получить проблемы при переносе системы (например с виндовс машины перетаскиваем в докер контейнер)

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

веселее может быть только этот конфиг потерять и потом со слезами на глазах восстанавливать.

Что значит «потерять» конфиг? Куда он денется из репозитория?
+5
Не знаю Go, но, как минимум, пересоздание массивов при каждом вызове метода выглядит подозрительно.

Почему вы решили, что массив пересоздают? Не знаю что там у go, а в сях этого массива вообще может не существовать после компиляции.

+1

А там массив или мапа?


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

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

А почему, кстати? Если она неизменяемая, её можно заменить чем-то вроде switch. Для какого-то количества включений в таблицу это даже может быть стоящей оптимизацией. Или там какие-то политические соображения?

0

Ну, во-первых, до C++14 компилятор вообще не имел права элайдить аллокации памяти, поэтому никто особо даже и не смотрел в сторону этих оптимизаций.


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


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

0
Premature optimization. В огромном числе программ пересоздание массива не является сколько-нибудь существенной проблемой. Это играет хоть какую-то роль только если приложение вызывает эту функцию сотни тысяч раз.
-1

Так можно оправдать любое неоптимальное решение, но только потом такие решения накапливаются и получается не очень. Современный софт часто тому пример. Если можно написать нормально, то почему бы этого не сделать?

0
Это вопрос баланса. В целом медленно работающий код сейчас лучше чем очень быстро работающий код через 5 лет. Где конкретно остановить «ползунок» на этой шкале определяется бесконечно большим числом факторов.
0

Я не понимаю, с чего вы взяли, что нормально — это с вынесенной константой? В некритичном участке кода читабельность гораздо важнее скорости, поэтому нормально — это так, чтобы связанный логически код не расползался по отдаленным местам программы.
В свете комментария (чуть нижев треде) о том, что это вообще toy-project на пару сотен строк, считаю этот спор вообще лишним, а исходный наброс — глупостью.

0
Не знаю Go, но, как минимум, пересоздание массивов при каждом вызове метода выглядит подозрительно.

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

-2
Я не разбираюсь в языке, но если придраться орлиным глазом, то
* перед фигурной открывающей скобкой есть/нет пробела…
* смущает наличие запятой после последнего элемента
* по-моему название одной из локальных переменных string одновременно является ключевым словом, что
* почему между первыми двумя массивами нет пустой строки, а перед simpleRegion есть?
+1
Я тоже не разбираюсь (только раз сходил в 2ГИС послушать про него), но в половине ваших претензий сомневаюсь:
* смущает наличие запятой после последнего элемента
многие языки это допускают без проблем (не добавляют пустой элемент в конце), а некоторые «стили программирования» явно это рекомендуют (чтобы при добавлении элемента в конец получался «дифф по существу» и не приходилось смотреть, только ли запятая добавилась в предыдущей строчке);
* по-моему название одной из локальных переменных string одновременно является ключевым словом
мне кажется, это всё-таки имя типа (в Go для меня несколько неожиданный синтаксис).

0
Я не разбираюсь в языке

Ну, так может для начала разобраться? ;)

* перед фигурной открывающей скобкой есть/нет пробела…

И не должно быть — так форматирует встроенный go fmt.

* смущает наличие запятой после последнего элемента

А она там должна быть — требования синтаксиса Go.

* по-моему название одной из локальных переменных string одновременно является ключевым словом, что

Нет там переменных с названием string. По крайней мере в пределах скриншота.

* почему между первыми двумя массивами нет пустой строки, а перед simpleRegion есть?

Это уже откровенная придирка, но даже тут у меня есть объяснение: вероятно, пробел отделяет декларацию мап (мапы тут по сути играют роль констант/статических данных, хотя они фактически и являются переменными; именно за это, кстати, код выше и критикуют).
0
* перед фигурной открывающей скобкой есть/нет пробела…
И не должно быть — так форматирует встроенный go fmt.

Ну так если не должно быть, то почему пробел там есть?

Опять же, я понимаю, что я придрался, но автор статьи прямо писала, что она докапывалась до мелочей, и свой код доводила до идеала.
0
Там он должен быть. Я неверно прочитал — думал про обычную скобку. Короче, код отформатирован с помощью go fmt и докапываться там не к чему.
+2
Тоже не совсем понял в чём проблема?
По моему, как раз удобно сделано. Берём регион (напрмер Франкфурт), по нему получаем название страны (Германия), а уже по названию страны берём её код. Довольно наглядно.

Плюс, если говорить про дальнейшую поддержку, возможно появится потребность получать код по другим регионам Германии, что будет сделано простым добавлением элемента в regionsToCountries. Если мапить напрямую регион — код, то наглядность исходного текста сильно упадёт.

Возможно, на GO можно как то изящней это сделать, я код на этом языке вижу первый раз в жизни )).
0
Ну как минимум то, что эти массивы пересоздаются каждый раз при запуске функции.
+1
Почему он должен это делать?
Бывают ситуации когда действительно каждый раз нужен новый массив, компилятор должен пропустить такое.
0
Бывают, но не так сложно увидеть, что эта структура никогда не изменяется.
+2

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

0

Хеш-таблица — не такая уж и сложная структура. Во многих языках достаточно написать static при объявлении, чтобы он не создавался каждый раз.

0

static это про другое, она просто создастся только при первом обращении, но тогда нужно помнить про thread safe.
А я имел в виду compile time вычисления, чтобы в рантайме она вообще в ДКА посчиталась или еще что нибудь прикольное.

0

Про thread safe у static-переменных надо помнить только начиная с C++11, что вы потратите пару циклов при каждом входе в функцию на потокобезопасную проверку флага инициализированности.

0
нужно помнить про thread safe

В нормальных языка не нужно — оно для каждого треда своё создаётся.


А я имел в виду compile time вычисления

enum вместо static и будет compile time


чтобы в рантайме она вообще в ДКА посчиталась

Конечный автомат предполагает кучу условный переходов. Едва ли это будет быстрее подсчёта хеша.

+1

Свой (неконстантный) static без аннотации thread-local для каждого треда — это не очень нормально.

+9
Вас не смущает, что добавление нового региона, страны или маппинга между ними потребует входа в код, изменения захордкоженных параметров и снова деплой на сервер? А если менеджер попросит 15 раз в день добавлять по региону? Это будет цепочка: создание ветки, внесение нового региона, PR, его проверка, тестирование кода, потом кат в дев, потом кат в прод. И рано или поздно таки будет опечатка и это уедет в прод, это лишь вопрос времени.

Эти параметры надо вынести в конфиг и читать 1 раз при старте приложения.

Что будет если кто-то подаст в функцию 2 элемента или абракадабру? Код дальше поедет исполняться, так как нет обработки и проверки. По хорошему в go надо выдавать ошибку, чтобы снаружи знали, что что-то пошло не так. В противном случае потом неделю будешь искать и править «кривые» данные в базах и соседних сервисах.

Результаты тестов штатным бенчмарком:

Её код:

BenchmarkGetRegion-8     5000000               249 ns/op


После выноса переменных:

BenchmarkGetRegion-8    30000000               39.8 ns/op


Код почти в 7 раз производительнее. Да, компилятор go не умеет пока что, оптимизировать такое.
0
Спасибо, теперь более понятно.
Согласен, хардкодить справочные значения не очень хорошо.
Про попытку передачи неправильного значения, в процитированном вами коде непонятно, так как там часть метода и непонятно есть она дальше или нет (в git нет, это точно).
+34
Вне контекста разбор одной функции ничего не говорит. Возможно менеджер просит добавить новый регион раз в пятилетку, а вы ускорили в 7 раз функцию, ускорив всё приложение в целом на 0,01%.

UPD1: Ещё я могу предположить, что аллокация слайсов внутри методов сделана специально, что бы эти переменные не торчали в области видимости всего пакета. Возможно вместо переноса переменных достаточно объявить их внутри методов как const, но в целом это проблема скупости языка.

UPD2: зашёл в репу, коду 4 года, описание репы «This is a toy project». Весь проект, в котором «менеджер попросит 15 раз в день добавлять по региону» занимает 138 строк.
+14
А если менеджер попросит 15 раз в день добавлять по региону

Значит надо бить его по голове веслом, за организацию управления командой или единственным разработчиком. В мире победившего Electron борьба за оптимизацию ценой ухудшения читаемости выглядит крайне смешно.
0
Не повторяться; ссылаться, а не копировать; single source of truth — это как раз про читаемость. Наши мозги работают именно так, книги написаны именно так
+1
книги написаны именно так

1 2, 3 4 5 6 7 8 9 А Б В?

«Ссылочность» в этом случае у нас полностью в голове, а вот для читаемости используются именно копии всего, от букв до слов и тезисов.

1 — да
2 — неужели
3 — вы
4 — правда
5 — представляете
6 — себе
7 — что
8 — книги
9 — написаны
А — вот
Б — именно
В — так
0

Вот именно что буквы и слова, а не рисунки объектов и прочего. Типа, слово — это пойнтер на класс объекта или действия, который для всех (примерно) един :-)
Тезисы тоже будут подкрепляться ссылками на источники, если они не широко общеизвестны.

+1
Я более высокий уровень имел в виду. Уровень, где существуют «вышеописанный», оглавление, номера страниц и даже абзацев, «см. рис. 5», «как описано в работе John Dow», abstract из научных работ (представляющий собой по сути .h — декларацию того, что изложено в подробностях дальше), саму манеру изложения материала в научной и учебной литературе (сначала леммы, доказательства теорем, определения, затем их использование как уже известных и доказанных понятий).

И лично для меня магические константы и структуры данных внутри кода — дикость. Я не делаю так даже в самых «наколеночных» bash-скриптах или одноразовых программках. Потому что говнокод начинает замедлять тебя (не только написание и изменение, но и чтение) уже в течение часа, а именно в таких вот «наколеночных» программках часто важна скорость изменения и гибкость и поэтому там важно писать чистый код, как ни странно это может кому-то показаться.
0
Такой уровень у меня как-то совсем не ассоциируется с ссылочностью. Я бы это сравнивал скорее с модульностью. Ссылочность — это оглавления и списки использованной литературы, штуки весьма неудобные для постоянной работы с ними.
+11

Ради попытки экономии на спичках вы создаёте зависимость от глобального конфига (что само по себе мешает тестам) или отдельный конфиг, который планируете инициализировать каждый раз (даже когда функция не вызывается). Вы не сэкономите 200 наносекунд, а потеряете.


Можно сделать lazy-конфиг, но это не тривиально.


Потенциальные преимущества перекрываются такими же потенциальными недостатками.


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

0
У каждого свои ассоциации на слово «конфиг». Для кого-то это JSON, для кого-то .ini, для кого-то — структура данных внутри кода.
0
Если мы говорим не о поделке на поиграться полчаса, а о бизнес приложении то:
изменения в конфиге стоят гораздо меньше чем изменения в коде: меньше времени девелопера, тестировщика, ресурсов CI и тестирования, меньше деплоя и отката на продакшене, все это стоит денег, и иногда больших денег.
Так что все что может быть вынесено в конфиг должно быть вынесено в конфиг, однозначно.
0

Никогда этого аргумента про тестирование при изменении конфига не понимал. Если вынести все в конфиг — то он становится кодом. И изменения в нем точно так же может сломать все то же самое, что эквивалентный код. Так почему тестирования меньше?

+1
Представьте себе апп с конфигом из двух переменных, конфиг — текстовый файл на сервере, вы протестировали его с двумя переменными и всевозможными значениями.
Чтобы переконфигурить апп нужно поменять значение переменной в конфиге и перезапустить апп, оно уже было протестировано на первом этапе.
Чтобы изменить код — нужно открыть тикет, найти девелопера, он поменяет код и запушит в ревью, ревью посмотрят несколько человек, запушат в репо, начнется билд и тесты, создается среда тестирования, бегут тесты, билд машина пыхтит, все работают. Потом если нет CD ждем пока релиз, выкатываем, обнаруживаем что что-то пошло не так из-за совсем другого бага, откатываем, тестим…
Ситуация понятна?
0

Нет. Принято решение, что изменение конфига можно не ревьювить и не тестировать потому что "тут уже все протестировано". Почему того же решения нельзя сделать про изменение этих же двух переменных в коде?


Или, в обратную сторону. Если изменение двух переменных в коде признается достаточным, чтобы все перетестировать, то почему изменение тех же двух переменных в конфиге таким не признается?

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

Но тогда позиция "все что может быть вынесено в конфиг должно быть вынесено в конфиг" нуждается в уточнении в смысле того, что именно может.


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


Как раз потому, что код мы через тесты и прочую бюрократию прогоняем и это долго. А конфиги изменять быстро. Поэтому в какой-то момент в язык конфигурирования добавляется возможность быстро поправить логику.

0
Чтобы переконфигурить апп нужно поменять значение переменной в конфиге и перезапустить апп, оно уже было протестировано на первом этапе.

А потом бац! И ваше приложение падает, т.к. в одну мапу вхождение добавили, а в другую — нет.

0
все что может быть вынесено в конфиг должно быть вынесено в конфиг, однозначно.

Точно ли это истинное утверждение? Проверьте его на количестве секунд в минуте. Это может быть вынесено в конфиг.


При этом в константу вынести можно, в конфиг — бессмысленно.

0

Ну там, кстати, leap seconds же надо как-то учитывать. Так что в конфиг надо выносить не количество секунд в минуте, а полноценный конфигурационный язык (или, по крайней мере, с условиями).

+5
сеньор не будет заниматься преждевременной оптимизацией кода
+2
Код почти в 7 раз производительнее.

Так, может, вообще на ассемблере всё переписать? Знаете, во сколько раз производительней будет?
0

Вынос в константы однозначно нужен. А вот вынос в конфиг может оказаться и оверинженерингом. Про YAGNI тоже не нужно забывать.

+1
Вас не смущает, что добавление нового региона, страны или маппинга между ними потребует входа в код, изменения захордкоженных параметров и снова деплой на сервер?

А вас не смущает, что реализация подобной подсистемы конфигов — это отдельная задача, которая должна быть отдельно согласована с тем самым менеджером? И что потом этот код с конфигами придется тестировать и поддерживать?


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

0
Да бог с ней, с оптимизацией. Есть вещи поважнее.
Скажем, преждевременная нормализация {«DE», «fra», \U0001F1E9} -> {«DE», «fra»}, {«DE», \U0001F1E9} — это очередной подводный камень при попытке обновления этого кода. Тут и так опечататься раз плюнуть.
А о том, что вообще такое это \U0001F1E9, я и задумываться лишний раз не хочу, это трата времени. Впрочем, сеньорам виднее.
-1
ахаха, ко всему можно придраться но код всё таки чище многих реальных якобы крутых проджектов. хотя бы читается, карл, не зная языка — а это уже что то. самое быдло тут перехват ошибок обычными условиями if err != nil вместо механизма эксепшнов, который везде должен быть. Мелочи — разный стиль формирования строк:
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")
вспоминаю очень строгого препода, который говорил что все константы, строковые в том числе, должны быть извлечены наверх проги
мож что то ещё тут, подозреваю что слишком специализированный код но я не знаю это уго
+5
Хорошо, что перед тем, как стать джуниором, я немного побыл лейтенантом…
+10
Окей, тест на лейтенанта.

Какой способ быстрее решит задачу:

1) Дернуть сержанта, объяснить задачу и сказать, чтобы он выводил бойцов на работу.

2) Вывести бойцов, объяснить бойцам, что от них требуется, произнести мотивирующую речь и отправить на работу.

3) Вывести бойцов, отдать приказ приступать к работе. Когда в толпе появится возмущение, ударить самому борзому по морде и риторически спросить «Кто еще против советской власти?»
0
Погоди Выполнять — Отменят.

Кроме того, задачу нужно решить не «быстрее», а в точно заданное время.
0
Вот хотелось бы добавить про «качество кода». Все используют эту фразу, но ни кто не может объяснить критерии качественности. Больше того: бывает, что тот код, который один сеньор считает эталоном качества, у другого не пройдёт код-ревью с резолюцией «переписать полностью».
0
Больше того: бывает, что тот код, который один сеньор считает эталоном качества, у другого не пройдёт код-ревью с резолюцией «переписать полностью».

Один из них как минимум делает что-то не так, а возможно даже оба.

Вместо того чтобы спрашивать «где хорошо», спросите лучше «где плохо и как это мешает?».

Метрики вещь сложная и неоднозначная, но имеют место быть в дополнение.
+16

Все очень просто.
1) Не выделяй память, если можно не выделять
2) Знай какие операции блокирующие, а какие нет. Очевидно читать файл или слать запросы в сеть в гуёвом потоке дело неправильное.
3) Не городи абстракций в одноразовых решениях или на среднем уровне архитектуры приложения. Конкретный код, под конкретное бизнес решение, никогда не будет переиспользован.
4) Соблюдай стайл-гайд
5) Если пишешь костыль, оставь огромный комментарий с цепочкой размышлений, который привёл к появлению костыля.
6) Если условие сложное, разбей на несколько булевых переменных с нормальным названием и скомбинируй. Если невозможно (например сложная регулярка), то оставь понятный текстовый пример, когда условие срабатывает и когда не срабатывает.
7) Не используй редких английских слов в названиях
8) Погугли хоть раз в жизни " best practices".
9) Не пиши сложно и оптимально, если код вызывается раз в час/сутки/жизнь.
10) Даже если ты невероятно крут и умен, помни, что читать и поддерживать твой код будут люди менее компетентные. Это не значит, что надо писать хуже, но значит, что в каждой точке, над которой пришлось подумать и сделать неочевидно — нужно оставить комментарий.

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

Качество кода обычно вещь всё же более сложная и обширная.
+1
Ну соблюдение единого стиля и именование переменных это уж совсем базовые вещи которые должны быть очевидны

Ох, если бы… :D Я проревьювил сотни тысяч строк кода :) Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных. Причем, чем более опытный разработчик, особенно если опытнее тебя, тем больше получишь в ответ надменных ответов в стиле "я так всегда писал, мне так удобнее".


Конечно, я сильно утрирую, но факт остается фактом — в 99% ревью, которые я смотрел нарушается по крайней мере 2 правила из этих 10 :)

+3
Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных.

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

+1

То, что содержание важнее формы совсем не отменяет важность формы, если мы говорим о качественном коде. Код выполняющий свою задачу — это рабочий код. Конечно, он лучше не рабочего. Но если его невозможно читать и если в нем "ничего не значащие пробелы" вообще не вписываются в структуру кода во всём проекте — этот код НЕ качественный. Даже если он супероптимизирован (возможно без причины), но понять его можно только после недели разглядывания и сделать это могут полтора человека в компании — это НЕ качественный код.


Ревью, к слову, не призвано к улучшению работоспособности кода. Предполагается, что код присланный на ревью УЖЕ работает. Ревью как раз призвано улучшить качество и как побочный эффект — увидеть пару багов :)

+2
Зачем вы тратите своё и чужое время на исправление пробелов и отступов? Вы ревьюте сотни тысяч строк кода и распыляете своё внимание на очень важную ерунду. Если с оформлением всё так плохо, то пора настроить линтеры.
+1

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


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


Я не говорю, что фанатичность в этих вопросах — это самое главное. Просто форма, в вопросах качественного кода имеет не малый вес.


В целом, весь этот спор похож на спор о важности секса в отношениях :) Да, секс — не главное, но без секса хорошие отношения с партнером для меня не существуют (субъективно).


Так же и соблюдением стайл-гайда. Рабочий код можно плодить и без него, но качественный — никогда.

0
Офис так не выглядит. А вот код без стайл гайда после многолетних правок десятками разработчиков — да.
+1

Если офис так не выглядит, то может дело не в стайлгайде?

0
даже со стайл-гайдом — в проекте, которому 5-й год идёт, с 10 разработчиками, когда я вижу код то могу часто сразу сказать кто конкретно его писал
+2

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


ИМХО.


  • Комментарии в коде часто недостаток, чем польза. Чаще всего они содержат //todo, либо //fixme, либо // это переменная. Если требуется комментарий, то надо подумать, а нельзя ли было написать лучше. Чаще всего, код документирует сам себя. Выбирай грамотные названия переменных. Комментарий ставится только тогда, когда без него никуда.
  • Даже если код вызывается редко это не отменяет необходимости сделать его оптимально.
  • Абстракции на уровне бизнес логики не менее важны, чем на уровне поддерживающего слоя.

UPD: Комментарии нужны для того, чтобы описать почему здесь сделано именно так, а не иначе, а не для ответа на вопрос что здесь происходит.

+1

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


Даже если разработать простой стайл гайд с простыми правилами, невозможно автоматизировать разрабов называть переменные содержательно и нормально описывать условия и заставлять оставлять атрибуты к функциям/переменным или проставлять const / noexcept у методов/переменных.


Чаще всего, код документирует сам себя.
Я такого не видел за всю карьеру. Тем более на С++ :) Тем более на старом С++. Однако при соблюдении всех гайдов, особенно по части названий, к этому можно приблизиться.

Комментарии в коде часто недостаток, чем польза
Хреновые коментарии, написанные не по инструкции — да. Правильные комментарии, которые я описал в п.5, которые отвечают на то, зачем и почему этот код тут — еще ни разу не портили код.

Даже если код вызывается редко это не отменяет необходимости сделать его оптимально
Зависит от того, хотим мы в итоге выпустить продукт или нет. Если не хотим, можем писать оптимально хоть всю жизнь :)

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

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

+1
Проблемы со стайлом должны решаться линтером автоматически. Я даже не обращал бы внимания на такие замечание: что-то не нравится? — пиши линтер рул и мы его обсудим. А так — смотри на функционал, а не на стайл.
0

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


Десятки правил неавтоматируемы, но совершенно легко понимаемы человеком.

0

Все все все это должно быть линтер рулами, все это прекрасно автоматизируется

0
Можно мне ссылку на инструмент, который это умеет делать с С++ кодом? Я бы не против его прикрутить.
0
Не знаю как С, на питоне мы используем AST, не говоря уже о куче поделок на его основе.
Неужели для плюсов ничего не придумали?
0

Питон синтаксически раз в 20 проще плюсов :) Для плюсов мне известен только clang-format и IDE-специфичные плагины, но все они неспособны выдать форматирование, которое я привел в качестве примера ниже.

0

А он-то тут вообще каким лесом? Это инструмент, по сути, статического анализа, а не форматирования кода.

0
Самые частые замечания именно по части несоблюдения стайл гайдов и названия переменных.

Стилевые проблемы обычно решаются добавлением линтера, которые есть для большинства языков, и хуков на коммит/мердж.
А названия переменных — вообще однa из сложнейших задач в программировании :)
[UPD] не обновил комментарии…
-1
Найдите мне хоть один способ автоматически получить вот такое выравнивание в коде С++, а так же проверку всех моментов, отмеченных в комментариях и я брошу все попытки доказывать, что автоматизация стайл гайда — дело почти нереальное:

/// @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, которые мне будут всё это проверять автоматически при коммите.
0
шланг-формат ограничен, хотя если сразу его использовать, то привыкаешь и над форматированием задумываешся намного меньше
а автоформатирование шаблонных портянок всегда очень скудное
0
Я в курсе того, что если с самого начала всё делать правильно, то всё будет хорошо. К сожалению, кто-то так не сделал, а стандарты существуют и отменить их не всегда возможно, по крайне мере в коде, который уже написан.
0
//Я не смог заставить ни одну IDE выровнять эту скобочку правильно

Так пусть скобка эту вверху будет. В чем проблема?

+1
Все очень просто.
1) Не выделяй память, если можно не выделять

Вы написали «просто», а потом сразу же влепили пункт, который мягко говоря не прост. Экономия по памяти достигается или расходом процессорных циклов, или (когда до этого еще не дошло) усложнением кода, когда можно бы и не усложнять. Даже очевидная свалка в памяти, в духе «храним одно и то же несколько раз в виде копий» — далеко не всегда однозначное зло, если избавление от этой свалки увеличивает сложность и связность кода, а объёмы даже в худших случаях таковы, что волноваться по этому поводу не стоит.

Вопрос о том, сколько и где выделять памяти — он, мягко говоря, очень непростой.
0

Я не говорю, что задача выделения памяти простая. Там настолько дофига нюансов, что можно книгу и не одну написать.


Я говорю о критериях качественного кода. Программист, знающий нюансы выделения памяти — напишет более качественный код, нежели программист их не знающий и выделяющий каждый байт в куче. Знание этих нюансов — это то, что мы называем опытом и то, что отличает код джуна, от кода миддла и сеньора.

0
Те самые знаменитые: «Пишите код так, как будто сопровождать его будет склонный к насилию психопат, который знает где вы живёте» и «Отсутствие комментариев в коде служит достаточным поводом для увольнения программиста с работы».
+1

Обычно код пишут так, словно у них дома склад оружия и маленькая армия вокруг :)

0
А комментарии, наоборот — как будто за каждую букву в комментарии с разработчика вычитают деньги из зарплаты :)
0
«Отсутствие комментариев в коде служит достаточным поводом для увольнения программиста с работы»

Какие именно комментарии желательны тем, кто так говорит?

-1
Из самого вашего вопроса я делаю вывод о том, что вы не понимаете смысл комментариев в коде. Это плохо. Но, к сожалению, весьма распространено (особенно среди тех, кто никогда не возвращался даже к собственному коду 3-5-10 летней давности, не говоря уже о коде чужом — собственно это один из столпов на которых зиждется подход «Да тут всё нужно к чёртовой матери переписать!»).
Хорошие комментарии могут содержать много очень полезной информации:
— почему сделано так, а не иначе
— какие допущения были приняты при написании этого кода
— какие изменения в будущем предполагаются автором
— какие детали на момент написания коды не были достаточно уточнены
— и т.д. и т.п.

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

но это несколько упрощённая точка зрения. Комментарии объясняющие «что здесь происходит» (то есть, по сути, дублирующие код) не только бесполезны, но и вредны так как являются той самой очень быстро устаревающей документацией которая, к тому же, запросто может оказаться изначально неверной (когда автор написал в комментарии одно, а в коде, по ошибке или неопытности, реализовал, по сути, не совсем то, что написано).
+1

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


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

0
Но может быть и наоборот. Когда комментарий правильный, а код — нет. Так что различие комментария и кода — это как различие кода с тестом. Если не сходится, то нужно разобраться и что-то из двух поправить. А если комментария нет, то ошибка в логике происходящего никогда не найдется.

Это не «наоборот». Это ровно тот сценарий, который я описал. В целом, конечно, написание комментариев не меньшее искусство чем написание кода. А может быть и большее (по моим наблюдениям писать более-менее работающий код умеет в разы больше людей, чем разумные комментарии)
+1

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


почему сделано так, а не иначе

Что именно вы вкладываете в это? Я вот, например, пишу парсер CMake-файлов по грамматике на сайте. Там точно нужны комментарии?


какие допущения были приняты при написании этого кода

У меня были случаи, когда подобные вещи описывались в виде отдельного TeX-документа, с формулами и их выводами. Но это ИМХО тоже нужно не везде.


Или мы, опять же, говорим о разных допущениях.


какие изменения в будущем предполагаются автором

А это зачем писать?


какие детали на момент написания коды не были достаточно уточнены

Ну, да, TODO для этого, и заодно описание, почему какие-то костыли были сделаны, возможно, с ссылкой на багрепорт — да, бывает. Но таких комментариев как раз чем меньше, тем лучше.

0
Что именно вы вкладываете в это? Я вот, например, пишу парсер CMake-файлов по грамматике на сайте. Там точно нужны комментарии?

Мне видится ровно два вариант:
1) Код настолько тривиальный, что комментарии в нём действительно не нужны (менее вероятно)
2) Комментарии, таки, нужны.

У меня были случаи, когда подобные вещи описывались в виде отдельного TeX-документа, с формулами и их выводами. Но это ИМХО тоже нужно не везде.

Или мы, опять же, говорим о разных допущениях.


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

А это зачем писать?

Ну, в моём мире редко когда приложение пишется сразу в полном и идеальном виде. Обычно всё начинается с PoC, а дальше развивается. Возможно в вашем — по-другому. Но, опять же, сильно сомневаюсь.

Ну, да, TODO для этого, и заодно описание, почему какие-то костыли были сделаны, возможно, с ссылкой на багрепорт — да, бывает. Но таких комментариев как раз чем меньше, тем лучше.

TODO — это вообще не про это. Оно скорее имеет отношение к предыдущему пункту. Про ссылки на баг-репорты тут уже кто-то (явно имеющий хороший опыт) рядом написал (про смену баг-трекера и тому подобные вещи).
+1
Мне видится ровно два вариант:

Ну это для любого кода применимо так-то.


Допущения — это те исходные позиции, которые явным образом не зафиксированы в коде, но были обговорены или додуманы при его написании.

Ну вы просто заметаете под ковёр некие определения. Вот «явным образом не зафиксированы» — это как?


Если у меня есть клиент для некоего внешнего API, и API требует отсылать не больше 2 запросов в секунду, то передать параметр 500ms куда-то там в менеджер очереди запросов — достаточная фиксация?


Если у меня есть вон тот парсер выше, и у CMake-файлов есть три вида строк (unquoted, quoted, raw), и я выбираю не представлять информацию о том, какого именно вида была строка, в AST, а сразу парсить их в универсальное представление — это надо фиксировать как-то?


Обычно всё начинается с PoC, а дальше развивается. Возможно в вашем — по-другому. Но, опять же, сильно сомневаюсь.

И в моём тоже. Но обычно дальнейшие планы формализуются в виде списка фич где-то в JIRA, багтрекере TODO.md или на доске маркером, а не в виде комментариев в коде. И формализуются они в достаточно общем виде, типа «реализовать то», «добавить поддержку сего». Да, я могу оставлять в коде комментарии типа // TODO generalize for arbitrary time windows или -- TODO better error handling.


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


Про ссылки на баг-репорты тут уже кто-то (явно имеющий хороший опыт) рядом написал (про смену баг-трекера и тому подобные вещи).

Ну, из всех ссылок, которые у меня были, багтрекер протух только у одного проекта (у кутей, ибо ссылки были ещё с тех времён, когда они были продуктом Trolltech). И даже в этом случае всё решилось подстановкой тех же циферок ID бага, просто на другом домене.


А что делать-то, в конце концов? Копировать описание бага в комментарии? А зачем?

0
Далее моё личное мнение, которое запросто может «не совпадать с мнением редакции»:

Если у меня есть клиент для некоего внешнего API, и API требует отсылать не больше 2 запросов в секунду, то передать параметр 500ms куда-то там в менеджер очереди запросов — достаточная фиксация?

Нет, недостаточная. Почему именно 500, а не, скажем, 600? А вот комментарий вида «API требует отсылать не больше 2 запросов в секунду» — достаточная.

есть три вида строк (unquoted, quoted, raw), и я выбираю не представлять информацию о том, какого именно вида была строка, в AST, а сразу парсить их в универсальное представление — это надо фиксировать как-то?

Разумеется. Иначе ваш последователь будет ломать голову: «На кой чёрт этот умник стал парсить именно в универсальное представление — это же требует в три раза больше памяти и совершенно не нужно в данном случае?!»

дальнейшие планы формализуются в виде списка фич где-то в JIRA, багтрекере TODO.md или на доске маркером, а не в виде комментариев в коде.


Ага. Потом с доски всё стирают, с JIRA переходят на Redmine (попутно теряя кучу данных) и все такие: «Так, мы же, вроде, хотели что-то там улучшить, но что?! Кто-нибудь может вспомнить?»

часть базы модифицируется вне транзакции

Так, на всякий случай — работа с БД всегда идёт в транзакциях. Если вы не управляете ими явно это не значит что их нет…

А что делать-то, в конце концов? Копировать описание бага в комментарии? А зачем?

А нет никакого универсального рецепта. Это элемент искусства, в некотором роде.
0
Ага. Потом с доски всё стирают, с JIRA переходят на Redmine (попутно теряя кучу данных) и все такие: «Так, мы же, вроде, хотели что-то там улучшить, но что?! Кто-нибудь может вспомнить?»

А может же и метеорит упасть на все машины с исходниками. Что тогда делать? Ведь комменты удалятся все.

0
А может же и метеорит упасть на все машины с исходниками. Что тогда делать? Ведь комменты удалятся все.


Тогда ничего не делать. Довольно странно размышлять о проблемах сопровождения кода, который уже не существует…
0
А я вот считаю, что термин «комментарии» давно устарел. В языках ассемблера, FORT или APL надо было комментировать каждую строчку, а в современных ЯВУ комментировать необходимо условия (зачем), константы (почему именно такая) и хакерские приёмчики.
Но код должен быть документирован. Самодокументрованный код подобен атомной бомбе: как атомная бомба всегда падает в свой эпицентр, так и самодокументрованный код всегда соответствует своей документации, какие бы опечатки не содержал. А кто умеет писать без опечаток? Особенно, копипастить.
Поэтому, документация к коду нужна. Можно держать её в отдельном текстовом файле, можно – прямо в тексте программы. Я совмещаю оба подхода. Пишу алгоритм в текстовом файле, после тщательной проверки копирую текст в Visual Studio, превращаю его в комментарии, а уже под комментариями расписываю код программы.
Да, с годами код изменяется. Комментарии около кода всегда правлю, чтобы код им соответствовал, а вот исходный текстовый файл описания часто остаётся без изменений, увы.
Благодаря этому подходу одна из моих разработок развивается (и продаётся) уже тридцать лет. Поменялось несколько компиляторов и ОС, только разрядность ОС менялась дважды. Если бы я не комментировал алгоритмы, то не смог бы так долго поддерживать программу в рабочем состоянии.

Конечно же, к программам — однодневкам всё вышесказанное не относится.
0
9) Не пиши сложно и оптимально, если код вызывается раз в час/сутки/жизнь.

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

+3
11. Добавляй метрики.
12. Добавляй логи.
13. Думай о типобезопасности и дружественности к рефакторингу.
14. Но знай, когда это всё (вообще это всё) не нужно.
+7
Потому, что качество кода — это то, насколько хорошо код выполняет свою задачу. А задачи у кода могут быть разные (и их может быть больше одной, с разными приоритетами).

  • Есть код, который должен выпоняться как можно быстрее.
  • Есть код, который должен занимать минимум памяти.
  • Есть код, который должен легко читаться.
  • Есть код, который должен легко читаться новичком/учеником.
  • Есть код, который должен легко читаться человеком без знания английского.
  • Есть код, который должен легко отлаживаться.
  • Есть код, который должен легко модифицироваться.
  • Есть код, который должен просто ожидаемо отработать один раз, и тогда все остальные пункты не имеют значения, а единственным критерием качества является отсутствие багов.
0
слова даже не сеньора, а инженера
согласен.

Но при работе в команде требуется все-таки пункт с хорошей читабельностью — а значит, приходится придерживаться каких-нибудь правил. Да и если даже пишешь в одиночку — удобнее возвращаться к старому коду, когда он написан так, что ты и через год его легко узнаешь и понимаешь
0
Но при работе в команде требуется все-таки пункт с хорошей читабельностью

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

Почему я считаю понятной запись
if(~x.indexOf(y))

?
Потому что её считают понятной все, кому придётся читать мой код.
0
Мы как-то всей командой возмущались, что лид любил так писать
Да, всем понятно, но никому не нравилось. Сравнение с -1 читается быстрее и проще.
+1
сравнение с -1 вернее еще и идеологически, потому что это не же результат битовой арифметики, а индекс (а еще я видел в одном старом лигаси библиотеки, где подобный вызов «дайте индекс объекта в коллекции» мог возвращать и -2, если не было ни точного ни «неточного» совпадения. Но это уже совсем другая история ..)
0

Вот это очень правильный способ рассуждений. Применение битовых операций к типу индекса — ну такое.

0
В вашей команде — не нравится, в нашей — нравится. А пишет-то каждый для своих, так, как договорились.
0
Есть код, который должен легко читаться человеком без знания английского.

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

0
— 1C (или если код дальше передаётся на поддержку человеку, который только на нём и писал).
— Обучение детей (школьный возраст).
— Встречал код китайской команды, у которых даже в названии одной переменной была шутка на китайском про псов-неудачников. Геймдев, продукт с миллионными заработками (AutoChess).

Но в целом, да, встречается не часто.
0
В порядке шутки:
  • Есть код, который НЕ ДОЛЖЕН легко читаться человеком, без знания национального языка команды разработчиков
0
ни кто не может объяснить критерии качественности

Это не проблема кода. Что такое "качественная книга" тоже никто объяснить не может. Это фундаментальное свойство качества.

+3
Я запускала ssh на сервере и делала git pull. Уверен, что никогда не видела ни одного пулл-реквеста.

Во-первых, — перебил его Филипп Филиппович, — вы мужчина или женщина?
+3
Хотелось бы обратиться к коллегам с просьбой — в документации, и комментариях к коду писать не только о том, ЧТО здесь делается, и КАК это работает (все это и так видно по коду), но — главное — ПОЧЕМУ именно так сделали.
Например, почему здесь mutex, а не InterlockedExchange, и тому подобное.
-2

Как можно заблаговременно ответить на вопрос "почему именно так?", если даже не представляешь "а как иначе-то?".

0
Поддерживаю, либо прямо в коде, либо ссылками на конфлюенс. Сам всегда так делаю, и остальным советую.
0
Я пишу заметки в коде для себя, когда делаю что-то неочевидное. Просто знаю, что пройдет месяц-другой и я понятия не буду иметь почему я так сделал. Но на тот момент были обьективные причины. Эти «хлебные крошки» очень помогают восстановить контекст принятого решения.
0
мне кажется, такое надо в командах обсуждать — ситуации очень разные
кому-то покажется странным и someArray.map->filter-> вместо цикла
а кому-то — цикл вместо цепочных функциональных вызовов

Имеется в виду, в разных командах — разные ситуации, когда может возникнуть вопрос «зачем». Есть вещи, про которые вообще никто не спрашивает. А теоретически можно спрашивать про все. Почему не использовали полифилл? Почему использовали полифилл? В команде надо принять свод правил — и придерживаться его (ну и плюс линтер, конечно, он лишним не бывает)
+1
По 5-му пункту — Хорошо, когда есть время всё документирвать. Вероятно у ваших предшественников его было меньше или не было вообще (т.к. дэдлайн по проекту — сдать вчера), следовательно все было сделано «на отвяжись». Удивляться не чему особо — все предсказуемо, и ребята здесь не причём.
+2
С документацией проблема не в том, что «нет времени», а в том, что время на документирование должно выделяться постоянно, а иначе документация устаревает со страшной силой (код меняем, документацию править некогда) и начинает только мешать вместо помощи.
-2
Мечтаю когда-то проверить идею наличия в команде отдельного сотрудника — технического писателя, который регулярно опрашивает коллег о том, что сделано, смотрит код, и пишет историю развития проекта и его модулей ( с обоснованием выбора решений).
Он же мог бы и за пробелами и табуляцией в коде следить, создавая скрипты форматирования.
+2
Такой писарь будет действовать исключительно как раздражающий фактор — будет ходить и донимать кодеров, чтобы те сливали ему документацию. Ценность такого сотрудника околонулевая — как сказали ваше — проще выделить время (притом в рабочее время), чтобы каждый кодер документировал коментарии ко всем изменениям за день — но это из области фантастики.
0
Даже, если он ничего не будет записывать, а только будет подходить с разговорами о работе, то как минимум, "эффект утенка" уже будет :)
И это действительно работает.

А еще — он станет центром обмена опытом:
подскажет как удобнее настроить среду разработки, и какие вспомогательные скрипты облегчат работу,
подскажет новому сотруднику — с чего начать знакомиться с проектом, где точки входа. Подскажет руководству о проблемах, на которые жалуются работники (где нужно закупить ПО/обновить оборудование/внедрить какую-то технологию/ликвидировать организационно-бюрократический барьер, мешающий работе).

Общаясь со всеми, он видит всех, понимает их настроения, знает их возможности и реальную загрузку.
Поэтому, сможет вовремя заметить профессиональный конфликт в коллективе, например, когда разработчик категорически несогласен с архитектором, и вынужден делать реализацию архитектуры, с которой категорически не согласен — на «отвяжитесь».
В этом случае, «ТехПис» подскажет руководству повнимательнее рассмотреть этот проблемный технический вопрос, с привлечением мнений различных специалистов.
Часто бывает, что люди обладают более широкими знаниями, чем нужно в их текущих задачах.
«ТехПис» поможет свести между собой специалистов для обмена опытом. Подскажет кто что может знать, с кем можно посоветоваться.
Чтобы стать таким человеком, нужно уметь не только грамотно и понятно выражать мысли (что само по себе — уже отдельный ценный навык, который развит не у каждого программиста), но и разбираться в программировании и технологиях, а так же — в психологии.

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

Другое дело — программистов там было не очень много (<20), а когда их сотни, то непонятно как масштабировать этот подход.
0
подскажет как удобнее настроить среду разработки, и какие вспомогательные скрипты облегчат работу,
подскажет новому сотруднику — с чего начать знакомиться с проектом, где точки входа. Подскажет руководству о проблемах, на которые жалуются работники (где нужно закупить ПО/обновить оборудование/внедрить какую-то технологию/ликвидировать организационно-бюрократический барьер, мешающий работе).

Общаясь со всеми, он видит всех, понимает их настроения, знает их возможности и реальную загрузку.
Поэтому, сможет вовремя заметить профессиональный конфликт в коллективе, например, когда разработчик категорически несогласен с архитектором, и вынужден делать реализацию архитектуры, с которой категорически не согласен — на «отвяжитесь».… подскажет руководству повнимательнее рассмотреть… проблемный технический вопрос, с привлечением мнений различных специалистов.… поможет свести между собой специалистов для обмена опытом.… Чтобы стать таким человеком, нужно уметь не только грамотно и понятно выражать мысли (что само по себе — уже отдельный ценный навык, который развит не у каждого программиста), но и разбираться в программировании и технологиях, а так же — в психологии

По описанию похоже на скрам-мастера…
0
Да? Может быть. У нас таких нет, и не было.
Мне кажется, очевидной необходимость в такой деятельности, как бы ни называлась должность :)
+5

Я не представляю ситуации, когда после двух часов написания сложнейшего костыля (именно о таких и речь в п.5), у тебя не найдется еще 3-10 минут на написание комментария к нему. Ничем, кроме как ленью оправдать это не получается. К легким костылям — комментарий обычно тупой: "здесь ставлю флаг, потому что без него падает проверка в функции Х".

0
в моей практике в глухом лигаси вместо длинного комментария — оставляли простую строчку вида КОГДА, КТО, НОМЕР ТИКЕТА. А уже в тикете будет все расписано более основательно, со скринами, логами и т.д.
+4

Видал и такое. Только потом оказалось, что компания переехала с TFS на JIRA и ссылки уже года 4 как не валидны :) Ровно как и ссылки на вики.


К тому же, даже если ссылка валидна — там описана обычно только проблема, а не её решение. В решении обычно всем лень писать что-то осмысленное, поэтому там написано "пофикшено в версии х.у.з".


А даже если и написано что-то, не факт, что под рукой есть доступ к корпоративной сети (например, я могу быть в поездке).


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


P.S.
Кто и когда git blame расскажет, нет нужды особо это писать. Да и вообще дело бессмысленное. Скорее всего, если ты забрался так глубоко — человек уже 100 лет назад уволился или просто забыл, что вообще писал этот кусок.

0
хорошо, когда код под git-ом, а когда это перфорс или еще что-то подревнее и в этом месте это -надцатая правка?

Понятно, что это все такой лигаси, что джуников туда не пускают. и пусть даже автор уволился, но можно будет найти потом через 10-20 лет (да, пришлось как-то править код из 1994 года) и локализировать его правки в то время (а будущим саппортерам — мои нынешние правки).
0

Неужто в тикетах там настолько подробно всё описывалось, что ими можно даже было заменить комментарии? Всё же тикет это обычно сущность более высокого уровня, чем построчный комментарий с разъяснением, почему решение именно тут не оптимальное или выглядит необычно.
Если нет — это может быть даже хуже чем нормальный комментарий, хоть и на целую страницу, потому что "КТО" — это bus factor в чистом виде, он может уже уволился, или банально делал это всё в кофейном бреду и уже ничего не помнит.

0
это такой лигаси, что там CVS древнее svn'а. Но дата+имя дает потом возможность найти правки автора в других файлах и посмотреть, что же он там такое делал (даже если тикеты протухли).
0
У нас тоже такое пошло. Но чужие задачи смотреть нельзя ))
+4
На редкость толковые наблюдения. Не уверен, что в виде статьи много людей это осознают. К сожалению такие вещи доходят преимущественно с опытом.
+2
Тут дело в том, что научить этим вещам — действительно не представляется возможным. Баланс между идеализацией кода (всё круто написано, но толком ничего не делает) и идеализацией решений (всё работает, но без валерьянки код читать никому не удаётся) — это, например, вообще только с опытом вырабатывается. И остальные пункты в принципе такие же.
+3
Согласен, я бы половины не понял 13 лет назад, когда начинал, и статья с большего пролетела бы мимо. Ну или согласился бы, вроде логично, но «хардварно», чутьем, до меня это не дошло бы. А сейчас могу пустить скупую слезу и только.

На практие жена — джуниор-вайтишник, 30 лет. Последовательно наступает на одни и те же грабли, что и я много лет назад, и мое присутствие и постоянные код-ревью мало что меняют. Каждый должен пройти свой путь :)
0
Так что представьте моё удивление, когда в первый рабочий день в стартапе я не нашла никаких тестов. Никаких тестов в интерфейсе. Никаких тестов в бэкенде. Вообще никаких тестов.
Нахрена в стартапе тратить время на написание тестов, если на следующей неделе фаундер с просветленными глазами прибежит с «пивотом» и весь код проекта следующее 3 недели придется заново переписывать.

На очень ранних стадиях отказ от всего что не является абсолютно необходимым для достижения результата (тесты, документация, навороченный таск менеджер) является прагматичным решением. Тесты нужно начинать внедрять тогда, когда овчинка стоит выделки: команда разростается и начинает заниматься в том числе поддержкой кода, а не только постоянным дописываением и переписыванием с нуля, требования к системе устаканиваются, возникает архитектура, а цена ошибок выростает.
+1

В усреднённом случае, любая команда начинает заниматься поддержкой кода сразу же после первого commit'а, а не когда-то в абстрактном будущем когда она "разрастается".


Если переписывание кода с нуля не сопровождается переписыванием с нуля требований к системе, то тесты — это наоборот спасительная соломинка, которая поможет хотя бы убедиться, что часть предыдущего функционала по-прежнему работает. Или узнать, что что-то поломалось — причём, узнать быстро, а не когда придут обиженные клиенты (ведь не нанимать тестировщиков в стартап — это тоже прагматичное решение, правда?).
А если сопровождается — то такова жизнь, тесты тоже код, и их придётся выкинуть в ту же самую яму. Но и тут — выкидывают ведь не всё, что-то остаётся нетронутым, а кто будет проверять работоспособность оставшихся фич? Программист за $300/час, или какая-нибудь автоматическая билд-система за $10/месяц?

0
А тестировщики и не спасут. Не могут же они до посинения тестировать всё снова и снова после изменения чего-то в коде. Ибо изменение в одном месте может вылиться в косяк в другом — абсолютно неожиданном — месте. Тестировщик может даже не знать, что такое влияние есть.
0
Не могут же они до посинения тестировать всё снова и снова

Вот вам смешно, а у нас в среднем японском айти всё так и работает, поменял код — бери лист в экселе, проходи всё и заполняй :/

0
Не могут же они до посинения тестировать всё снова и снова

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

0
Вы невнимательно прочитали весь контекст обсуждения. Речь шла о разработке без автоматических тестов.
0

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


У них работа, знаете ли такая — тестировать всё снова и снова, до посинения, да. И чем меньше покрыто автоматическими тестами — тем сильнее загружены тестировщики. Обратное, впрочем, как правило, неверно.

0
Переписывать больще половины кодовой базы проекта из-за изменения требований это уже, извините, не изменение требований, а новый проект получается.
+1

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

0
Ваша правда.
Думаю, в таком случае тут важнее тот момент что часто тесты не появляются и с развитием стартапа, по крайней мере те тесты, что пишут разработчики(unit)
+2

Переводчику для справки.


"Nada" из оригинала != "Нда".
Это "нда" здесь сильно выбивается из остального ряда (см. "Ничего. Ноль. NaN. Тесты отсутствуют как явление").
Nada = nothing. Перекочевало из испанского в американский английский.

Only those users with full accounts are able to leave comments., please.