Это история о плохом коде, который однажды написал ваш покорный слуга.
На одном из первых курсов университета я писал программу на Java, которая должна была читать файл весом в 6 MB в строку (этим файлом был геном бактерии в формате FASTA). Выглядел мой код следующим образом:
BufferedReader reader = new BufferedReader(new FileReader (file));
String line = null;
String text = "";
while( ( line = reader.readLine() ) != null ) {
text = text + line;
}
Построение строки при помощи серии конкатенаций подобным образом крайне неэффективно — у меня, без преувеличения, уходило около 40 минут на чтение файла (с тех пор я узнал несколько способов получше). Самое главное — после чтения файла весь оставшийся алгоритм в программе отрабатывал секунд за 10. Два дня я так и работал: делал изменения в коде, запускал программу и успевал посмотреть целый эпизод LOST, прежде чем программа завершала выполнение. «Черт, на двенадцатой строчке ошибка! Опять все по-новой...»
После множества повторных запусков я наконец подумал «наверняка должен быть лучший способ сделать это». Я выяснил, что можно написать цикл на Perl, который сможет считать геном менее чем за одну секунду (при этом на Perl я умел программировать не лучше, чем на Java — просто повезло). Итак, я сел и написал скрипт на Perl, который читал файл, собирал его в одну строку и выводил ее. Затем я сделал так, что моя программа на Java вызывала скрипт на Perl через командную строку, захватывала вывод и сохраняла его в строку.
Если бы у меня сохранилась эта программа, то я бы напечатал ее исходный код и повесил в рамку на стену, чтобы почаще себе напоминать о том, что никогда и никого не буду ругать за написанный им «плохой» код.
Рассказать об этом случае из моей жизни меня вдохновил свежий комикс с XKCD про “качество кода”:
Комикс кроме как «злым» не назовешь — опытный программист не дает ни одного полезного совета, только проводит гиперболические аналогии. Но что больше всего зацепило мой взгляд, так это ответ пациента — «Хорошо, я почитаю руководство по кодинг стайлу». Отреагирует ли так живой человек, если кто-то будет настолько груб к нему? Разве ответ будет похож не на «Хорошо, это был последний раз когда я показал тебе свой код», или даже «Хорошо, думаю я должен завязать с программированием»?
Проблема присутствует и в научных исследованиях. Есть много причин, по которым ученые публикуют свои статьи без прикладывания к ним кода (и они вполне объяснимы), но первую позицию в их списке обычно занимает смущение: «мой код слишком уродлив для того, чтобы отдавать его кому-то». Те, кто стремятся пристыдить других за код, в этом вопросе не слишком-то помогают!
Известный совет начинающим писателям гласит: «Каждый из нас держит в себе миллион плохих строк. Прежде, чем этот миллион выльется наружу, пытаться написать что-то хорошее бесполезно. Так идите и пишите!». Я вспомнил о нем, когда читал дискуссию, разгоревшуюся по поводу приведенного выше XKCD:
The only way to write good code is to write tons of shitty code first. Feeling shame about bad code stops you from getting to good code
— Hadley Wickham (@hadleywickham) 17 апреля 2015Когда вы, опытный программист, в следующий раз захотите пристыдить кого-нибудь за его код, попробуйте мысленно вернуться к собственному миллиону строк кода. Представьте, что кто-нибудь осудил вас так же, как и я себя выше. Продолжили бы вы просить о помощи? Осилили бы вы свою дорогу в миллион строк?
Комментарий Джона Кармака
Не надо путать слова «написавший этот код идиот» и «этот код дерьмо» — между ними есть существенная разница. Комикс как раз о последнем. Предполагаю, что сделано это сознательно. Атака на автора вряд ли принесет какую-то пользу, но должна существовать возможность строго критиковать сам код. В реальной жизни, у многих авторов не получается отделять себя от своей работы, однако умение проделывать это — ценный навык.
Предоставленные сами себе, большинство людей демонстрирует непробиваемую способность игнорировать свои недостатки, и это вредит их профессиональному росту. Небольшой стыд часто становится позитивной мотивацией. Мне стыдно за огромное количество из того кода, что я написал за последний год. У меня в голове есть мысли насчет причин, почему все получилось так, как получилось, — некоторые из них представляют собой оправдания, но часть из них — это чистой воды «WTF я думал вообще, когда писал это?». И если вы не чувствуете небольшой вины за ваши последние работы, то, вполне возможно, вам будет полезно, если кто-то укажет вам на проблемы — в том плане, что этот человек сможет пробиться сквозь ваши «защитные механизмы».
Я был бы счастлив, если бы кто-нибудь «зарылся» во весь тот код, что я написал для Oculus Mobile SDK. Уверен, что по большей части этим читателем рано или поздно стану я сам, и по большей части буду только кивать головой и соглашаться с написанным; однако, я уверен, что смогу вынести из этого что-то полезное, и это благотворно повлияет на мою дальнейшую работу.
>> Вы в Oculus делаете код ревью для каждого коммита?
Нет. У нас есть некоторые подвижки в этом направлении; мне интересно будет увидеть, что из этого получится. На нас все еще сильно сказывается давление темпа работы над стартапом.
>> Ваш комментарий к статье выглядит так, будто макака прокатилась по клавиатуре, а вы потом только прошлись для виду автокорректом… ну как, прозвучало не слишком уж полезно, правда? Думаю, что вы путаете излишнюю жесткость с серьезной критикой. Заметили, что моя последняя фраза не такая обидная, и в то же время принесла толку больше, чем первая?
Обычно тактичные и полезные комментарии работают лучше всего, но бывают случаи, что необходим некоторый минимум «активационной энергии» для того, чтобы комментарий произвел действие на человека.
>> Делали ли вы ревью кода, написанного другими разработчиками в id в прошлом?
Неформально, и тогда мне этого хватало. Мне не нравится заниматься ревьюированием кода; возможно, по этой причине я высоко ценю критику.
Пусть Джон и признается, что отчасти не сторонится в жизни строгих комментариев, ему далеко до Линуса Торвальдса, щедро пересыпающего свои письма личными оскорблениями, а выступления — фразами в духе «Меня не интересуете вы, я беспокоюсь о технологии и ядре — вот что для меня действительно важно» и утверждениями, что «никто не услышит вас, если вы собираетесь быть мягким».
Комментарии (50)
boolivar
29.04.2015 13:23+19А меня прям бесит когда мой код ревьюжат те, чей код недалеко ушел от говнокода. И критика вида: че-та методов многовато и где джавадоки??
mapron
29.04.2015 23:51+5Меня как-то ревьюил опытный коллега, который у нас не работает уже. После этого я к ревью не обращался.
-Много двойных пробельных строк;
-Не отбиты пробелами вызовы функций;
-Не все методы с Doxygen комменнтами;
-Названия классов видите ли надо с литеры C начинать, а то вроде как непонятно где класс а где метод (у меня подсветка кода, мне все понятно).
Я спросил его «ну а архитектура-то как?» он «ну я хз, я не понял как оно работает. вроде норм.»
ncix
30.04.2015 00:17+1С претензиями к оформлению и именованиям можно смело слать лесом самого Страуструпа, если в конкретном проекте не утверждены соответствующие стандарты.
MacIn
30.04.2015 02:03+2Если это официально принятый code style, то претензии по делу — если во всем коде классы с С, а у кого-то нет, читать трудно.
По поводу форматирования у нас поступили просто — есть автоформатёр. Если кто-то написал не так, можно просто текст выделить, нажать пару клавиш и готово, можно ревью по существу делать.
fetis26
03.05.2015 15:18+1Скорее всего ему было лень пробираться через дебри непривычных наименований. Если все оформлено по гайду, то ничто не мешает смотреть пл существу
mapron
04.05.2015 21:16+2Увы, хотелось бы чтобы у этой истории был такой позитивный конец, но нет. Очень мало людей хотят разбираться в сути чужого кода.
VokaMut
29.04.2015 13:33+2Через год-два после того, как я начал учить язык php я понял, что хочется посмотреть как пишут другие. И я полез на киберфорум и стал помогать новичкам решать их проблемы, мысленно критиковал код и наматывал на ус.
n0ne
29.04.2015 13:59+17Знаете, иногда уже фиг с той красотой кода, понять бы, что это такое я написал год назад…
StrangerInRed
29.04.2015 14:01+12Можете попробовать комментарии, есть такая фича во всех языках программирования. Правила наименования еще неплохая вещь.
burjui
01.05.2015 17:41+1Прежде, чем задаться целью писать комментарии, советую прочитать «Чистый код» Роберта Мартина (aka Uncle Bob).
zloddey
29.04.2015 14:29+8Тут вот такая загогулина получается, что код обычно делается «красивым» не для того, чтобы от взгляда на него получать эстетическое наслаждение, а чтобы его было проще читать и понимать. Возможно, само слово «красивый» наводит на ложные ассоциации? Попробуйте подставить вместо него слово «читаемый» — и тогда сразу же появится смысл.
n0ne
29.04.2015 15:59+8Видите ли, когда я открываю конспект по «вышке» от первого курса, а это 93й год, то я впадаю в ступор от того, какой я тогда был умный, если понимал всё это.
Бывает откроешь свой файл, ну вот все переменные понятно называются, методы, классы, всё понятно, но «угадал все буквы, но не смог назвать слово»: ничего не понятно, что, куда, откуда и почему… пока не проследишь, что откуда берётся, пока примерно не восстановишь ход своих _тех_ мыслей, не всегда бывает легко разобраться даже в собственном коде.
Flammar
29.04.2015 18:53Чаще бывает нужно восстановить ход не всех _тех_ мыслей, а только их части. Тогда «красота» кода, в смысле разбиения на блоки, хорошо помогает.
khim
29.04.2015 20:55У меня нет в мыслях «разбиения на блоки», уж извините. Я вот сейчас регулярно комментирую изменения, которые в носятся в один модуль созданный мною 4 года назад. Понять тот ужас, который там сейчас имеется я могу с большим трудом, но я ведь всегда могу взять свою оригинальную версию с функциями длиной по 200 строк (не преувеличение… их там, правда, не так много) и поиграться с ней. Ну а потом, уже зная что и где нужно менять я могу и придумать как завернуть всё в ту версию, которая там сейчас используется (на это обычно уходит несколько больше времени, чем на первичное решение проблемы, но так как решение я уже знаю, то это не так страшно).
Зато «красиво»: убрана дубликация кода (как будто эти 100 строк копипаста могли кого-то убить), есть куча комментариев и тестов и т.д. и т.п. Ну и? Кому и зачем всё это было нужно? До сих пор не знаю…
justaguest
30.04.2015 07:22+1В «100 строках копипаста» могли быть ошибки, которые, будучи найденными в, скажем, оригинальном куске кода, останутся в скопированном. Поверьте моему печальному опыту — это не круто, пофиксив такую ошибку, размышлять «Не скопировал ли я куда-нибудь еще этот код?»
khim
30.04.2015 08:03Мимо. Там были две идентичные функции для работы с двумя архитектурами. То, что в них могут быть идентичные ошибки и что в большинстве случаев их нужно править одновременно — как бы самоочевидно. Зато каждая из них была достаточно проста и было понятно что в них происходит. Сейчас же та же логика размазана по двум десяткам функций и для того, чтобы всё это не разъехалось пришлось написать не один десяток тестов. Где, понятно, тоже могут быть ошибки. Притом что за четыре года ни одной ошибки, которая могла бы проявиться на практике, в исходном коде не было найдено, а в «улучшенном» кода такие ошибки были — не могу сказать, что я прям в восторге от проделанной работы.
Притом из чисто теоретических соображений я понимаю, почему маленькие функции лучше и почему тестирование — это замечательно и т.д. и т.п. Вот только практически я этого эффекта не наблюдаю.
oberon87
29.04.2015 15:56+2Все подобные красивые и правильные рассуждения страдают от недостатка метрик и формальных определений. И это помимо всяких метрик сложности по памяти, по времени исполнения и т.д.
Куча разных критериев, удобство сопровождения, понятность для коллег по code review. Что такое читаемость, что такое наглядность, что такое выразительность? Какие-то субъективные критерии, которые легли на девственно чистый участок коры головного мозга авторитетного автора, пока он работал на первой быдлокодерской работе. Как всегда недостаточно весомые критерии для публичных рассуждений.
Кто-то пытается выехать на актуальных трендах, типа, stream вместо foreach, как раньше выезжали на foreach вместо while. Кто-то считает строки. Кто-то ветвления считает. Кто-то просто выдумывает красивые и звонкие мемы и аллегории, как на картинке.
В итоге все превращается в выяснение истины методом голосования.
nuald
29.04.2015 17:15+4К сожалению, даже метрики и формальные определения не всегда помогают. Вот, например, один из самых больных для обсуждения требование к стилю — длина строк не должна превышать 80 символов. Несмотря на то, что есть логические предпочтения для этого, абсолютное большинство голосуют против этого: «мы же не в каменном веке терминалов!!!» А то, что:
- вообще-то с терминалами до сих приходиться работать (SSH в AWS/другие облака),
- code review для длинных строк кода очень тяжело делать (потому что в основном это side-by-side comparison),
- некоторые разработчики предпочитают другие layout-ы на экранах — например, иметь консоль с логами от watcher-а (gradle watch, grunt etc) или в целом просто tail -f логами рядом с редактором кода
их не волнует. К сожалению, многие разработчики имеют практически религиозную фанатичность насчет некоторых практик и инструментов, и только голосование (или в худшем случае, когда даже это не работает, официальное требование от руководства) может повлиять на изменение стиля.
P.S. Хотел бы отметить один момент с layout-ами — многие возражают, что сейчас у всех много (2+) мониторов на работе. Тем не менее, современная практика разработки программного обеспечения (не уверен насчет России) включает в себя WFH (Work From Home), и дома у большинства нет лишних мониторов. WFH становится более и более популярным потому что это снижает риск эпидемий на работе (болеешь, можешь сидеть дома и работать, тем самым не теряя зарплату) и современные технологии практически исключили требования иметь всех под боком в любой момент времени.lany
29.04.2015 17:54+1Про 80 символов — сильно зависит от языка. В той же Java бывают имена классов по 30 символов. Если ограничивать по 80 в таких условиях (учитывая ещё отступы по 4 пробела), код начинает выглядеть откровенно уродливо. На перле 80 символов — вполне адекватно.
nuald
29.04.2015 18:25+2Естественно, не зря современный стандарт — 80/120 (и ситуация намного хуже для конфигураций, где, например, в том-же JSON-е из-за отсутствия многострочных литералов приходится иногда писать очень длинные строки). Я больше ссылался на разработчиков, которые имея возможность писать короткие строки, тем не менее пишут код длиной до 2800 символов в строке (реальный пример, я сам не мог долго поверить своим глазам и все попытки убедить, что так делать не надо, уперлись в каменную стену фанатичности: «я — snowflake, что хочу то ворочу, и никто мне не указ»).
ingumsky
30.04.2015 02:24+32800 в одной строке?! А чем автор кода мотивировал своё решение? У него вместо монитора бегущая строка?
fetis26
03.05.2015 15:26Касательно читаемости, на мой взгляд, есть очень простая метрика — любой код в команде должен выглядеть как написанный тобой. Это идеал, к которому надо стремиться
khim
03.05.2015 18:15То есть если Вася умеет реализовывать сложные математические алгоритмы, но не умеет обрабатывать сложные структуры данных, а Петя успешно обрабатывает сложные форматы данных, но у него плохо с математикой, то в коде не должно быть ни сложных структур данных, ни сложной математики? Ну и куда вы зайдёте с таким «идеалом»?
JIghtuse
29.04.2015 17:58+1Пусть Джон и признается, что отчасти не сторонится в жизни строгих комментариев, ему далеко до Линуса Торвальдса
Едва ли мы об этом узнаем. Разработка ядра Linux ведётся в списке рассылки, в котором каждый может принять участие в качестве читателя или писателя. Это одновременно и преимущество (открытость), и недостаток — появляются подобные суждения. Линус тоже говорил, что хороший пинок бывает полезен для человека, просто так он никого не ругал.
dyadyaSerezha
29.04.2015 20:05-22А меня задела в статье суперполиткорректная картинка, где девушка суперпрограммист учит жизни говнопрограммиста парня. Не видел таких случаев.
С нетерпением жду новых исторических сериалов, например, про Эйнштейна и его теорию, где он будет черной девушкой-феминисткой. А то неудобно как-то — великий ученый — и опять мужик? Дискриминация, понимаешь.dyadyaSerezha
30.04.2015 19:33-4Забавно, что ни один из минусующих не возразил: «нет, вот конкретно у нас была такая крутая девушка-программист». То есть, минусуют за сам факт, потому он неприятен? Ну заминусуйте еще за факт, что 99% научных открытий (проверить легко по статистике Нобелевских лауреатов) совершается мужчинами и что 99% детей рожается женщинами. Ну про 99%, это я политкорректно написал. В-)
saluev
01.05.2015 10:22+6Ну я даже не знаю, погуглите Мариссу Мейер там, или Аду Лавлейс для начала. А минусуют вас за то, что вы сексизм разводите на ровном месте.
justaguest
29.04.2015 21:17+3Мой первый — и длительный — серьезный опыт в программировании выглядел похожим образом, разве что ждать приходилось не сорок минут. У меня была программа на С++, написанная в С стиле, с использованием void указателей вместо темлейтов (что я аргументировал, фактом наличия таких указателей в функциях стандартной библиотеки С, а темплейты слишком ужасно выглядят).
Чтобы найти только одну ошибку, которую я бы мог обнаружить, если бы умел правильно писать код, мне приходилось готовый бинарник переложить в определенную директорию, переключиться на терминал, подключенный к устройству, откуда командой «tftp -gr file $IP» скачать файл, затем не забыть дать ему исполняемые права, затем переместить в другую директорию на замену старого приложения, затем убить старое приложение, затем подождать некоторое время, пока его перезапустит демон, затем на ПК в клиентской части приложения сделать еще несколько пассов, и получить сегфолт. Затем проделать часть манипуляций, связанных с запуском gdbserver, и подключением к нему, снова воспроизвести сегфолт, и, наконец, заняться изучением причин.
Но чаще всего сегфолта не возникало, а просто что-то работало неправильно, и все нудные манипуляции приходилось повторять полдня, чтобы только найти одну проблему.
Постепенно я понял, что надо писать код так, чтобы по максимуму ошибок можно было обнаружить на этапе компиляции. Не последнюю очередь в этом сыграло и изучение языка Haskell — который, кстати, весьма рекомендую людям не знакомым с ним: даже, если вам не придется работать с этим языком за деньги, он научит вас писать код действительно правильно, даже на других языках.burjui
01.05.2015 17:48+1Haskell нужно изучить ещё и ради функциональщины. Недавно с удивлением обнаружил, что коллега не знаком с функциональным программированием. Парадигма не является «серебряной пулей», но очень мощна, особенно в обработке коллекций.
ivlis
30.04.2015 05:04В науке это нормальное дело. У меня была программа на python, которая запускала программу на C++, которая читала вывод из Maple и потом передавала данные назад, а программа на python строила график. Даже если бы я к статье приложил этот код, не думаю чтобы он кому-то помог :)
Mixim333
30.04.2015 16:37Мне кажется, что автор абсолютно прав, а вот это: «Каждый из нас держит в себе миллион плохих строк. Прежде, чем этот миллион выльется наружу, пытаться написать что-то хорошее бесполезно. Так идите и пишите!» вообще бесспорно. Для меня, написание кода аналогично написанию стихов поэтом и лично мне дает огромное, если хотите, вдохновение, когда код, на который ты потратил свое время не только решает быстро сложную задачу, но и легко читается даже через год.
StrangerInRed
С радостью бы что-нибудь поревьюил в свободное время. Это и опыт для себя и взгляд со стороны. Чем круты opensource проекты — можно научится чему нибудь, или помочь кому нибудь, найдя баг.
EngineerSpock
Я недавно выдрал с проекта кусок архитектурный, который с проблемами. Если хотите — можем поревьюить вместе. Я не знаю, как его улучшить.
lany
Так к чему это «бы»? Что-то мешает? :-)
StrangerInRed
Не, ну я лажу по исходникам на гитхабе, когда есть время. Но так чтобы кто-то просил посмотреть, обычно никто не просит :)
ApeCoder
codereview.stackexchange.com
Myshov
У neovim'а есть пункт, связанный с ревью кода.