Comments 55
Теория вероятности говорит, что если у нас есть система из двух устройств с надежностью Х и У и для того, чтобы она дала сбой необходим сбой обоих устройств, значит общая надежность системы это 1 — (1-X)*(1-Y). Т.е. для «устройств» Вася и Петя с одинаковой надежностью 75% общая надежность будет 93,75%! Смотрите, количество багов уменьшилось не до 12.5%, а до 6.25%!
Правильные рассуждения, но неправильная аналогия применительно к коде ревью.
Действительно, если есть rand(1,4) rand(1,4) и ошибочной ситуацией считается две четверки одновременно, то шанс «ошибки» (двух четверок допустим) будет действительно 6.25%.
Однако! в случае с программистами, Петя может не только исправлять ошибки Васи (выкидывать не 4-ку), но и вносить ошибки в его код (каковую ситуацию вероятности в приведенной трактовке не учитывают), т.е. он может не только позитивно влиять на ситуацию «4-ка у Васи», но и негативно на ситуацию «1,2,3 у Васи». Понятно, что вероятность того, что Петя накосячит в коде Васи минимальна, но она есть:) А если Петя еще и немного авторитетный, а Вася немного ленивый — вероятность внесения новой ошибки весьма заметна.
Коротко это называется «вышла новая версия, удалены старые баги, добавлены новые»:)
На возможность Пети вносить ошибки в код Васи очень сильно влияет постановка процесса code review. Если это не правка кода в стиле «Да что же он тут понаписал, етить!» а взвешенные комментарии к коду — тут переменные непонятно названы, тут надо метод выделить, код дуплицируется, тут эти свитч кейсы лучше развернуть в маленькую иерархию классов, и эти комментарии к коду потом кратко обсуждаются — согласен\не согласен, и, если Вася согласен, исполняются, да еще финально проверяются Петей, то вероятность внесения ошибок Петей равно нулю.

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

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

любой из комментариев может потребовать, чтобы Вася слегка подрефачил, если он согласится. Нормальный процесс.
У всех разные отношения и понимание что есть Code Review :) Ктото считает что это когда второй Гуру говорит «То что ты написал это лажа и надо было написать вот так» Причем так как он гуру то первый должен обязательно следовать его советам.
К сожалению.
У нас на работе вот так и было, за каждым программером, как правило, закреплялся кто-то по опытнее. Нам это очень помогало. Часто тебе самому кажется, что твой код довольно таки хорош и устойчив, но свежий взгляд со стороны помогал выявить кучу скрытых проблем. Времени на это дело каждый день уходило не более 15 минут.
В надежности Васи и Пети уже заложены верочтности добавления ошибок каждым из них. Выражение вроде верное было описано, но выводы некорректные. Если ревьюит один человек, то багов уменьшится не в 4 раза, а в зависимости от надежности обоих. Что и видно в выражении (проценты фэйлов обоих перемножаются — вероятность совпадения двух событий, и отнимаются от единицы).
Если бы у Пети надежность была < 75%, то вероятное количество багов увеличилось бы. Если бы надежность Пети была бы > 75%, то вероятное количество багов уменьшилось бы. Т.е. НЕ ВСЕГДА один дополнительный человек в качестве ревьюера уменьшит число багов в 4 раза.
Две ошибки:
Не все программисты руководители (PM, TL, ...) хорошо знакомы с теорией вероятностией.

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

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

Есть хорошая книга на эту тему — ''Совершенный код"
Через две недели я с большой вероятностью буду думать также, как и сейчас. Максимум получится обнаружить глупые ошибки, типа недобавленных проверок на null и т.п. Увидеть недостатки выбранного решения удастся с малой вероятностью. ИМХО
Максимум получится обнаружить глупые ошибки

Подавляющее большинство ошибок именно таковыми и являются. Человеческий фактор. ИМХО
Я бы даже сказал чем Катастрофичнее выглядит баг, тем тупее ошибка.

Самый фееричный баг который я фиксил приводил к глухому зависанию и затем ребуту телефона в некоторых случаях и зывался поставленной точкой с запятой после пары десятков пробелов на строке вместе с for :)
Ну почему у меня в институте не было теории вероятности?
Сейчас бы оно так пригодилось…
Что мешает почитать самостоятельно? Там нет ничего запредельного и не так много времени потратить придется(разумеется для ознакомления и повседневного использования, а не на уровне академиков)
Абсолютно ничего, этим я и хочу в будущем заняться.
Для текущего моего проекта будет ОЧЕНЬ кстати)
Там нет ничего запредельного, кроме необходимости знания хотя бы основ функционального анализа :). Для понимания сути описываемого.
Но для простого применения формул по шаблону, да, всё просто и на пальцах.
Ну конкретно мне это нужно для составления формул для весового многофакторного рандома.
Термин мне не знаком, но, думаю, вы, таки, сможете обойтись без сигма-алгебр и прочего функана :). На этом уровне предмет довольно простой и понятный. Но изучать надо тщательно, возможностей совершить ошибку там просто море.
Ну т.к. знаний у меня в этой сфере не много, я не знаю, есть ли для этого общепринятый термин.
Мы делаем систему, которая показывает в псевдослучайном порядке (рандом) контент, причем псевдослучайность получается «умной», т.к. зависит от ряда факторов (многофакторность), каждый со своим весом.
Для её тонкого тюнинга хотелось бы как раз использовать эту теорию, ведь «на глазок» не всегда можно получить приемлемые результаты, хочется все теоретически просчитать и продумать.
Спасибо за советы!))
Очень странный институт, где на IT специальности не дают ТеорВер. Я бы не стал в таком учиться.
У нас курс назывался "Организация и технологии защиты информации", и IT-составляющая его была не так уж и велика. Все таки я вышел не "Инженером по ИБ", а "Специалистом по ЗИ", так что говорить что это IT-специальность никак нельзя, это очень сильное сужение направления.

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

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

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

Это значит неверное поставлен процесс провдения ревью. По-хорошему код комитится в продакшен только после ревью, правки замечания и подтверждения правки просматривающим. Более простая схема — код комитится сразу, но ревью закрывается (нужна система для учета ревью) только после всех правок и подтверждений.
Горели сроки и было ещё несколько весомых факторов, поэтому на львиную долю замечаний тупо забили.

Про систему учёта ревью — это верно, нужна такая.
посмотрите crucible. Он правда довольно тяжеловесный, но довольно удобный и эффективный.
Содержание статьи, кратко: при code review проверяющий может найти баги, которые пропустил автор кода. Все.

Все цифры, представленные в статье, основаны на абстрактной модели «Вася-Петя», которая далека от реальности, но приводят к выводу, выраженному в первом предложении этого комментария.
Соответственно, «расчитанная» эффективность основана на цифрах c потолка и в качестве реальной аргументации бессмысленна, т.к. при «честных» расчетах все опять сводится к первому предложению этого комментария.

Противники Code review его не любят не потому, что не знают тервер (который, ИМХО, в статье притянут за уши) и не могут оценить эффективность от проверки кода, а потому что, в основном, не хотят тратить ресурсы на чтение написанного кода, вместо написания нового.
Содержание статьи не в том, что «проверяющий может найти баги», а в том, что «проверяющий найдет больше багов, чем кажется». Цифры в статье, конечно, взяты с потолка, но общий тезис вычисления надежности системы по надежности отдельных её частей я считаю, вполне применим.
Ваш тезис заключается в том, что при расчете вероятности обнаружения бага надо не тупо делить 25% (условно говоря) пополам, а считать по более правильной (без иронии) формуле?

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

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

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


Таким образом, я считаю предположение «проверяющий найдет больше багов, чем кажется» в реальности эквивалентным фразе «проверяющий может найти баги», потому что не имеется такого субъекта, которому кажется, что проверяющий найдет N% багов.

Моя основная придирка заключается не в том, что модель, решение или выводы в статье неверны, а в том, что решается проблема, которой и нет. Лучше провести анализ «цена/качество/время» при наличии code review и без него, который действительно можно использовать для аргументации «за» или «против».
>потому что не имеется такого субъекта, которому кажется, что проверяющий найдет N% багов

Мне кажется, что такой субъект есть. Работа менеджера проекта в том и состоит, чтобы располагая какими-то человеческими и временными ресурсами получить на выходе рабочий проект. Т.е. он обязан примерно знать состояние качества кода и иметь какие-то примерные оценки способов его улучшения. Если такого человека нет или у него нет таких оценок, то Вы, конечно, правы.
Я не отрицал наличие человека, который обязан проводить оценки и пр.
Я лишь утверждаю, что этот человек не будет рассчитывать вероятность возникновения багов (по вине каждого отдельного программиста или команды в целом).
Оценки, которые он делает, в первую очередь касаются времени: сколько времени потребуется на выполнение задачи (проекта). Если это грамотный менеджер, то он просто примет как данность, что после выполнения задачи возникнут баги. Со 100% вероятностью. И он просто увеличит оценку на некоторое значение, исходя из собственного опыта, которое уйдет на отладку и фиксы. Кроме этого, он всегда будет держать в уме, что в будущем все равно понадобятся ресурсы для поддержки продукта и фикса найденных пользователями багов.

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

Отсюда возвращаюсь к «не имеется такого субъекта, которому кажется, что проверяющий найдет N% багов».
Я не раз видел оценки «количество багов за время» или «количество багов на количество строк» — в основном они делались для проектов военной\научной тематики (точно помню, что такие оценки делало НАСА). Если команда неизменна и уровень задач примерно тот же — оценку с прошлого проекта вполне можно перенести на следующий. А по ней уже вполне можно прикинуть и время — а это, как Вы отметили, и есть нашей целью.
А что скажете насчет оценки количество_багов/количество_новых_фич?
Плюсую аналогично опирались на метрики:
Количество замечаний на ревью на число строк кода/патча, количество найденых багов на число строк.

В итоге оценка давала основания более пристально посмотреть на ревью, было ли оно на самом деле или чисто формально поставили «ревью ОК», позволяла оценить качество. Примерно позволяла оценить качество QA (извините за каламбур)

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

Согласен с комментом xappymah, тервер притянут за уши, в реальности слишком много нюансов, которые в сумме все эти вычисления делают несоответствующими реальности.

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

Единственная правильная оценка эффективности ревью, на мой взгляд, это внедрить такую практику а через полгодика оценить (хотя бы эмпирически) дало оно эффект или нет. По своему опыту могу сказать, что ревью дает отличный результат, даже его проводят все на равных. В условных попугаях могу назвать цифру в +15% к эффективности работы команды.

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

Теория вероятности говорит, что если у нас есть система из двух устройств с надежностью Х и У и для того, чтобы она дала сбой необходим сбой обоих устройств, значит общая надежность системы это 1 — (1-X)*(1-Y). Т.е. для «устройств» Вася и Петя с одинаковой надежностью 75% общая надежность будет 93,75%!

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

Если брать пример с программистами, то это значит, что они должны допускать ошибки в разных и независящих друг от друга местах. А если у них примерно одна квалификация или схожие представления о, например, уязвимостях, то корреляция тут явно не нулевая.
По моим ощущениям, эффект от ревью сильно преувеличен.
Если сильный программист ревьювит слабого — да, польза очевидно видна за счёт того что находится масса ошибок и слабый быстрее учится.
В ситуации когда просто один программист смотрит код другого — польза скорее от совместного владения кодом, проверки того что код добавлен куда надо, плюс всякие мелочи такие как ревью code style.
Но для того чтобы ревью было действительно полноценным и находило существенные ошибки, ревьюверу нужно погружаться на тот же уровень что и автор, т.е. смотреть те же документы, проверить все условия, скомпилировать под все модели — в общем, потратить существенное время. Получаем что результат существенно зависит от затраченного времени и от того насколько ревьювер ответственно подходит к делу. Времени всегда нехватает — люди жертвуют глубиной ревью.
Вы вроде все правильно пишите, но на моей практике эффект есть даже если джуниор ревьюит тимлида. Ошибаются все, даже боги. А в своем коде заметить ошибку гораздо сложнее чем в чужом, это факт. Да, архитектурные ошибки может углядеть действительно только опытный ревьюер, но есть же ошибки мелкие, технические.
В теории вероятностей, когда говорится про несколько событий, всегда упоминается «при прочих равных условиях». Программист Вася, пишущий код ночью, и тот же Вася, делающий ревью кода на следующее утро — это два совсем разных программиста.
Это я к тому, что нельзя одного и того же человека рассматривать как статическую систему с одинаковыми характеристиками.
UFO landed and left these words here
На счёт количественной оценки качества:

Это шутка, конечно, но процесс Code Review уменьшает количество WTF при чтении кода.

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

Цифры конечно с потолка. Не с потолка сам принцип — опыт двух людей не суммируется, а перемножается. На этом принципе и Code Review держится и парное программирование и многое другое.
Пример: Вася знает, что где-то глубоко внутри кода используется сортировка и какой именно алгоритм применяется, но не знает о других алгоритмах и их эффективности. Петя знает эффективности разных алгоритмов сортировки, но понятия не имеет, что в коде вообще применяется сортировка.
По отдельности эти знания ничего не дают, но при их соединении может получиться артефакт «мы знаем как ускорить код за счет более эффективной сортировки».
По мне более вероятен случай выявления нетривиального кода, неверности документирования, отсутствия комментариев и т.п.

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

А тем более сложность кода — как раз когда ревью слабым разработчиком оказывается очень полезно — он покажет какие места не очевидны, а чем нетривиальней код тем вероятность косяка выше.
UFO landed and left these words here
>> Качество нельзя оценивать количеством

Очень меткое замечание. Именно этого и пытаются добиться все управляющие проектами менеджеры.
Можно, для этого нужнен опыт предыдущих разработок.
Единственное что не стоит делать из него «священую корову», но оценка на каких то данных, пусть не совсем верная все таки более достоверна чем гадание по времени полета плевка до потолка в кабинете менеджера :).
UFO landed and left these words here
>Я не знаю, какие Вы лично проекты делаете
Лично я не делаю а только участвовал и участвую. Но сложилось так что размеры кода измерялись в первом случае гигабайтами (но это была большая корпорация). В текущем же случае измеряется сотнями мегабайт, но надежность должна быть 24/7/365. Это вам ни какую нибудь социалочку замутить.

Проходили мы и agile и XP. Но в серьезных проектах важно не столько качество, сколько прогнозируемость его и прогнозируемость сроков. Нету программиста Пети и Васи.

Есть оцениваемая сложность функционала X человеко-недель, и n доступных ресурсов. Соответственно ожидаем готовность тестированию через X/n*Kz недель календарных с примерно Y% пройденых тестов с дополнительными затратами +Z недель и итоговым качеством 95%.
И вот как хошь так и вертись. Все новомодные методики хороши с начала. Но внедрить TDD в 4 Гига уже существующего говнокода на 70% писаному индусами…
И вот как раз правильная оценка сложности и коэффициента на сколько срок увеличиться и является основной проблемой при планировании.

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

И только команде интеграции не спалось все эти модули собирать воедино и заставлять этот гребаный телефон таки работать и не ребутаться если приперлась SMS, а юзер в это время в редакторе записной книжки сидел :))
У нас в компании есть две обязательные фазы проверки кода:
а) Персональный code review
б) Инспекция

(а) делает автор кода, (б) — два человека из своей (или даже чужой) команды.

Обе вещи полезны. Только по рекомендациям PSP (Personal Software Process) code review надо делать не сразу, а через несколько часов или на следующий день, чтобы, так сказать, «на свежую голову».

А на инспекциях у нас находят те дефекты, которые автор не видит. И такая схема успешно работает уже много лет.
Only those users with full accounts are able to leave comments. Log in, please.