Pull to refresh
Comments 44
>Но по сути то и там и там код — это всего лишь набор байтов, которые было бы полезно оценивать без привязки к авторитетности источника.
Так себе идея.

Слегка условный, но типичный пример — читаем из внешнего источника строку, и пытаемся преобразовать ее в число. Как назвать отстутствие проверки и/или обработки ошибки в этом случае? Бизнес-требования изменились? Да как бы не так — это внешний источник, и доверять ему нельзя никогда.

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

Я не призываю при оценке кода отказываться от привязки к авторитетности источника, я призываю оценивать дополнительно в отдельном потоке без привязки к авторитетности.


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

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

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

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

Даже опытный и матерый программист может допустить глупую ошибку в силу многих причин.

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

Если программист под рассмотрением может допустить ошибку, то этой ошибкой вполне может быть пропуск комментария. Т.е. утверждать на 100% как оно должно было быть вы уже не можете.
Слегка условный, но типичный пример — читаем из внешнего источника строку, и пытаемся преобразовать ее в число
Такое вполне может быть, если профилирование показало, что чтение с проверкой корректности снижает пропускную способность сервиса на пару гигабит в секунду, поэтому проверки сознательно убрали.
Не верю (с) Станиславский. Если вам все равно нужно число — вы так или иначе проделали преобразование. Обработка ошибок (если как правило их нет) обычно не может так снижать пропускную способность.
Хорошие проверки, это не только валидация символов, но и проверки на переполнение, что числа вмещаются в текущие int32/int64. То есть, больше ветвлений.

К тому же, код основного цикла обработки мог слегка распухнуть из-за десятков тщательных проверок и перестать влезать в L1 Instruction Cache.
Я специально написал выше «обычно». Сервисы, которые из внешнего источника читают данные, и при этом требуют оптимизации такого уровня — они вообще-то достаточно редки. Если же речь такой случай — то опытные программисты обычно просто оставляют следы, позволяющие понять причины того или иного решения.

Вот да. Недостаточно писать работающий код, надо писать понятный код.

Там, скорее всего, тормозить будет файловая система или база данных, а не проверки.
Я вообще-то в первом же комментарии его дал — это источник, которому нельзя доверять. Чем оно не устраивает?
Мне кажется, на вопрос в разделе «Умение разбираться в чужом коде», отвечает раздел «Стандартизация».

Хотя на мой взгляд стандартизация — это просто перераспределение страданий. Чем более стандартизирован код, тем больше степень страданий при написании, и меньше при чтении.

Оформление вообще не проблема, проблема это когда программист не стал утруждать себя чтением доков. Как может не раздражать, когда эндпойнт назван /api/user/createUser или все ответы, даже с ошибкой, имеют статус 200? Или когда код лежит архитектурно не в том месте? А зачем было открывать файл контроллера и писать там, если можно прямо в роутах оставить, ведь главное что работает и выполняет задачи бизнеса.

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

У меня никакого раздражения, я, будучи спокоен, как удав, пишу комментарий и жму request changes.

Как бы плохо и ужасно он не был написан, в конкретное время и в конкретном месте он успешно решал конкретную проблему

Да, например, проблему job security автора кода. Резко войду в положение и начну любить код (и его автора).


Некоторые из них более читабельны, чем другие — пример субъективной оценки.

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

UFO landed and left these words here

Или если имена переменных уже имели вид вроде a, a2, b_bvrg или bOld_aux2.

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


int someName(const int* a, int count) 
{
    int rv = 0;
    const int* f = a;
    if (f)
    {
        const int* t = f + count;
        for(;f < t;) 
        {
            if (!*(f++)) 
                rv = 1, break;
        }
    }
    return rv;
}

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


bool hasZero(const int* arr, int count) {
    if (arr == nullptr) 
        return false;

    for(int i = 0; i < count; ++i)
        if (arr[i] == 0)
            return true;

    return false;
}
UFO landed and left these words here

(зануда mode on) Только зря вы переносите операторы в for и if на новую строку без фигурных скобок. Уже не первое поколение программистов наступает на грабли, дописывая там ещё оператор.

Функция или переменная плохо названа — ctrl+shift+R и в пару секунд она называется по хорошему. Табуляции вместо пробелов,… — ctrl+shift+F…! Комментарий избыточен или устарел — ctrl+D и его нет.
О, отличные лайфхаки! А подскажите, с чем там надо Ctrl жать, когда есть файл на 6500 строк и там ряд функций, строк по 400 каждая, полученных путем копипасты друг из друга с небольшими изменениями, реализующими несколько различающуюся бизнес-так сказать-логику?
Где по 200 строк типа x= asd_ewdfgh(hgfd_trew_zxcvb(zyx::asdf_iuyt(qwe_rtyu.begin(), qwe_rtyu.end()) + ytre_qwe(zyx::asdf_iuyt(qwe_rtyu.begin(), qwe_rtyu.end()); с разными названиями переменных, но одинаковым смыслом, который можно вынести в функцию x= do_work(qwe_rtyu);.
По итогу файл ужимается до 300 строк (обычной длины) и чуть проще становится для понимания, но занимает это жуть как много времени, вот бы хоткей какой знать…

По своему опыту могу сказать что, например, Intellij прекрасно справляется с пониманием того что где-то есть такая же логика, но с другим названием переменной

UFO landed and left these words here

А почему я должен испытывать какие-то эмоции? Это обычный контроль качества. Я спокойно описываю необходимые изменения, аргументирую, и тыкаю кнопочку request changes.


Если разработчик воспринимает сухую аргументированную критику кода как личное оскорбление — это его проблемы, не мои.

Я обнаружил, что есть люди, которые на это обижаются. Их расстраивает что они вот старались-старались, писали код изо всех сил, а вы со своими сухими «здесь segfault при N=2», «не соответствует пункту 3.2 codestyle», «здесь race condition», «здесь поведение не соответствует указанному задаче», «это изменение приводит к проблеме, ранее исправленной в задаче JIRA-1234» заставляете их понять что их код плох. Дальше у них срабатывает логический переход «мой код плох -> я плох», ваши аргументы становятся аргументами в пользу этого, и они бегут жаловаться что вы такой-сякой бессердечный, жестокий и вообще токсичный.

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

Не нанимать людей, которые еще не повзрослели (ментально, а не физически).


Это еще на собеседовании сразу видно. Если не смогли разглядеть на собеседовании — на испытательном сроке точно проявится.

Пусть тогда и на компилятор обижаются, когда он warning-и выводит.


Детский сад, штаны на лямках.


Почему меня вообще должно волновать, старался человек или нет? Значение для бизнеса имеет только результат. Зарплату разработчик получает не за старания, а за выполненную с надлежащим качеством и в надлежащие сроки работу.


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

Хабр не поощряет таких комментариев, но все же напишу просто. Молодец!
Стандарт PSR ратует за 4 пробела вместо табуляции, игнорируя очевидный факт: среда разработки большинства PHP-программистов изменилась с консольных редакторов на полноценные IDE, в которых оперировать табуляциями удобнее во многих смыслах: и удалять проще, нажимая Backspace один раз, и настроить можно индивидуальную длину табуляции по вкусу.

Во-первых, в любой нормальной IDE это прекрасно настраивается.


Во-вторых, Developers Who Use Spaces Make More Money Than Those Who Use Tabs :)


Ну и в-третьих:


  • внутри команды разработчиков/группы единомышленников может быть какой угодно кодстайл (но зачем, если есть PSR-12?);
  • один разработчик может использовать какой угодно кодстайл (но зачем, если есть PSR-12?);
  • всё, что выкладывается публично, должно быть отформатировано по PSR-12, иначе лучей ненависти будет слишком много.
Затем, что удалять и добавлять один таб гораздо удобнее, чем 4 пробела. Про навигацию стрелками с пробелами я вообще промолчу.

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

Вы пропустили этот пункт?

Ок, перефразирую. Если IDE компенсирует недостаток какого-либо подхода, это не значит, что его нет.

Хотелось бы всё-таки услышать хоть какие-то разумные аргументы за пробелы.

В любой нормальной IDE нет никакой разницы.


Лично для меня нет вообще никакой разницы. Главное, чтобы в проекте был code style и все его придерживались. Какой именно — по барабану. Но если есть общепринятый стандарт, всегда выгоднее его придерживаться: если понадобится форкнуть какую-то стороннюю библиотеку, с вероятностью 99% она оформлена в соответствии с общепринятым стандартом, и не понадобится ее реформатить, что упростит последующие мерджи с апстримом.

Если понадобится одолжить чужую клавиатуру, то с вероятностью 99% это будет QWERTY :]

Купить вам клавиатуру — это копейки, а регрессии из-за сложностей с мержем могут бизнесу обойтись в миллионы.


Если вы пишете опенсорс или свой собственный проект, то, конечно, дело ваше. А если вы получаете зарплату, то ее вам платят не за ваши личные хотелки, а за то, что нужно бизнесу. А стандартизация всегда бизнесу выгодна, любое нестандартное решение это всегда дополнительный cost of ownership. Любое нестандартное решение должно чем-то компенсировать этот рост TCO. Нестандартный code style не компенсирует это вообще никак, хоть убейся.


(И, да, если вы печатаете вслепую, то вам все равно, какие где буковки написаны, а если нет, то хоть какая там раскладка, все равно плохо. :-)

Да пожалуйста, пробел везде = 1 символ на экране, табуляция — везде разная, где-то визуально она 4 пробела, где-то 8. Используя пробелы вы везде получите одинаковое отображение, хоть в IDE, хоть в блокноте, хоть в vim'е.
Тот же phpstorm штатно из коробки использует 4 пробела. При желании можно настроить на табы. Лень нажимать 4 раза бекспейс/delete? Есть Ctrl+Alt+L.
Я на самом деле поискал после этой дискуссии.

1. Некоторые редакторы вообще не способно нормально отображать табы.
2. При разных настройках отступов, съезжают блочные комментарии, сделанные табами (лол) справа от текста.
3. У кого-то код ревью с лимитом на длину строки не проходил из-за того, что у ревьюэра стояли более широкие табы.

Ну и Ctrl+Alt+L есть не везде, так что либо страдания в одну сторону, либо в другую.
Данный шорткат именно для шторма указал. Проблем с табами по факту больше, чем профита, на мой взгляд.
Only those users with full accounts are able to leave comments. Log in, please.