XP. Недопарное программирование (Code review).

    Все много писали про «Парное программирование». Как это клёво и всё такое. Но как бы возникает проблема, что два программиста за день пишут как бы 150% работы одного программиста. Ну то есть теоретически меньше.

    А вот у нас в компании было так, что просто за каждым коммитером (тот кто имеет право добавлять код в основную ветку программы) был назначен ревьюер и после коммита в trunk (основную ветку программы), тикет (да!!! каждый коммит должен быть сделан по тикету) переводился на ревьюера (в тикете писался номера коммито(ов) для этого тикета). Ревьюер отсматривал изменения и или переводил тикет тестеру или же возвращал его коммитеру по одной из причин: логическая ошибка, не соблюдение правил кодирования, сложный код который нельзя прочитать и он не задокументирован, ну или же он явно видел ошибку в коде (напр. забыл проэскейпиться).

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

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

    А у вас что-нить подобное использовалось/используется?

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

    Подробнее
    Реклама

    Комментарии 68

      +4
      Долго ридел текст силясь чтонибудь андрерстуд.
        0
        пятница?
        • НЛО прилетело и опубликовало эту надпись здесь
            0
            не… 10-е. Вот понедельник будет да!!!
            0
            Увы, я лишен такого удовольствия как пятница, так как суббота рабочий день.

            Я имел ввиду что вы пишете слишком много английских слов русскими буквами.
              –6
              бедненький
        • НЛО прилетело и опубликовало эту надпись здесь
            0
            Вы считаете?
            • НЛО прилетело и опубликовало эту надпись здесь
                0
                попробовал переписать.
            0
            Ну нормальный такой вариант (насколько я смог выловить суть). Только это не парное программирование, а дополнительная проверка специально обученными (и оплачиваемыми!) роботами, что каждое изменение в код должным образом документируется.
              0
              не «роботами», а именно таким же точно человеком который сидит рядом.

              Да и я не говорил, что это парное программирование, это некий такой компромисс и вроде как оба сидят программируют и каждый друг за другом наблюдает.
                +3
                Смысл парного программирования в том, что человек, у которого нет клавиатуры гораздо больше сосредотачивается не над тем, как и что пишут, а над тем, зачем пишут. В данном случае от парного программирования остается лишь то, что участников — двое.
                  +1
                  Еще раз перечитайте мой текст, где я сказал что это парное программирование? Я сказал лишь, что это альтернатива. Так же когда ты отсматриваешь код у тебя в этот момент нет клавиатуры и ты сморишь зачем же это написано, а не как. Я всегда смотрел зачем здесь вот это изменение.
                    +2
                    1. Пересмотрел еще раз.

                    >Программирование в паре. Упрощенный вариант.

                    это ваш текст?

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

                      0
                      1. извините в текст смотрел, поправил.

                      2. может я не так написал или вы не так поняли, но это точно такой же программист, обычно даже еще и умнее того кто написал, но можно и того же уровня.
              +4
              Вообще-то, это нормальная практика, которая называется Code Review и ничего общего с парным программированием не имеет. Есть даже достаточно мощные тулзы специально предназначенные для этой практики, которые позволяют автоматически рассылать код перед коммитом на оценку старшим девелоперам (которых может быть несколько), и только после получения подтверждений коммитить.
              Эффект от парного программирования в том, что когда долго и жестко кодишь, иногда залипаешь на чем-то простом, вроде «что же тут за условие в коде должно быть». Два мозга вместо одного с такими задачами справляются быстрее.
                0
                что бы не вводить в заблуждение — я переименовал топик.
                  +1
                  Тогда можно чуть подробнее про организацию процесса. Есть ряд таких проблем с этой практикой — вот сидит человек, пишет, а тут бац, ему код на ревью присылают. Ему сразу надо отрываться, терять концентрацию. Или наоборот другой перегиб — писал, писал человек, закоммитил. А через день видит, что не прошел коммит и ему вспоминать, что он там вчера накодил. Интересно кто как делает, на самом деле. )
                    0
                    Отсматриваешь когда у тебя есть время. То есть не теряешь над своей задачей концентрации.

                    А вот если через день к тебе пришёл твой коммит, который не прошёл до тестера, то ты должен посмотреть и исправить, а если ты через день не можешь вспомнить что ты там написал, то представляете как в коде будет разбираться человек, который через месяц будет это поддерживать, а через год? Значит вы такую фигню написали. По хорошему день назад тебе достаточно посмотреть на код и ты вспоминаешь, а вот если дольше, то уже только если хороший код.
                      0
                      Посмотрел, бросил текущую задачу, пошел исправлять, отправил, потом вернулся, дописал предыдущую задачу, опять отправил, от ревьюера вернулись обе + поступила новая задача…

                      ИМХО, такая параллельность ни к чему хорошему не приведет. В данном случае лучше действовать последовательно. То есть не надо, ожидая результатов проверки, начинать новую задачу. А проверка может и не быть быстрой — ревьюер занят!
                        0
                        Отсюда напрашивается вопрос — а не будет ли потеря 50% времени одного программиста (200% — 150%) — меньшим из зол? Может, лучше бы сидели они вместе за одним компом…
                          0
                          Я же предположил что нет. Объяснения к предыдущему комменту дал.
                          0
                          Если вам больше одного раза возвращается — то вы очень плохой программист, у нас такого не было скажем так.

                          То есть вы реально считаете, что если вы отправляете свой тикет на тестирование (без ревью) и вам возвращают это плохо? Ну в смысле лучше сидеть и ничего не делать и ждать пока тебя оттестируют? вот это вообще медленно и потеря времени будет. К тому же когда с ревью возвращается это обычно подправить код или написать документацию, что не занимает очень много времени.
                            0
                            Резюмирую свою позицию. Я бы не стал применять такую практику, потому что:

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

                            2. Сам процесс очень прерывистый, слишком частые переключения с задачи на задачу

                            3. Слишком сильные ограничения на компетентность первого программиста: если ему вернутся более чем 2 задачи (от тестеров или ревьюера), и он работает над третьей, то возникает мозговой коллапс

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

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

                              3. У вас никогда не было больше 4-х тикетов? Да ну не поверю, а висит он и висит, делаешь когда придёт время.

                              4. Концентрация наоборот выше. Ты всегда в готовности.

                              З. Ы. Мне кажется вы не понимаете смысла ревью.
                              0
                              Чорт! Рус, ты только что назвал меня плохим программером:)))
                              ЗЫ у меня было несколько тикетов таких;))
                              ЗЗЫ пойду упьюсь:)
                      0
                      Когда мозг залипать начинает, надо встать, походить по комнате, клавиатуру из рук выпустить) У меня почему-то решения быстрее в голову приходят, если не сидишь за компом и в код смотришь, а ходишь туда-сюда:)
                        0
                        Очень интересно! А что это за тулзы такие? Как называются? Можно линки на них?

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

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

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

                        Соответственно, на лицо нехватка автоматизации. Ясно же, что через Subversion все эти изменения получить можно. Рассылка по емейлу изменений в чекине — неплохой шаг. В общем, хотел бы узнать как это в других компаниях работает
                          0
                          Навскидку не скажу.
                          Можно погуглить code review tools.
                          Можно подождать. kriconf.ru/2008/index.php?type=info&doc=speech_archive — тут по идее когда-нибудь должен появится архив с КРИ-2008. Там как раз должен быть доклад нивала на тему code review и других практик, которые они используют — докладчик много тулзов называл, для Java/Jidea.
                            0
                            > Очень интересно! А что это за тулзы такие? Как называются? Можно линки на них?

                            Мы такое вот-вот откроем на reviework.com как публичный сервис
                          +1
                          Идея интересная.
                          Главная проблема, как со всем новым — чтобы прижилась в коллективе.
                            0
                            У нас прижилась.
                            0
                            последнее слово предпоследнего абзаца

                            P.S. Без претензий, опечатка, бывает, просто заметил
                              0
                              спасибо, поправил.
                              0
                              А у вас что-нить подобное использовалось/используется?

                              однажды пользовался с удаленым программистом
                              collabedit'ом
                              • НЛО прилетело и опубликовало эту надпись здесь
                                  0
                                  Не знаю, когда я просто так пишу и когда в мозговом штурме у меня не на много производительность повышается. Может я в простом режиме хорошо пишу, а может в мозговом штурме у меня мозги не включаются на много процентов, но реально процентов на 20 повышается.
                                  • НЛО прилетело и опубликовало эту надпись здесь
                                      0
                                      не знаю… Может вам работа не нравится, но у меня из 8-ми 6-ть точно получаются полноценными рабочими часами. Ну это в среднем, бывают и простои.
                                  0
                                  А вывод и мораль где в топике? На прошлой работе у меня использовался Code Review, дисциплинирует, однако человек с опытом лажу комитить не будет в любом случае :) Штука полезная, но не всегда. Я бы предпочел использовать автоматизированные средства типа юнит тестов для того, чтобы проверять функционал на регрешены, нежели вот такой вот упредительный контроль. Мелкие ошибки все равно просачиваются :)
                                    0
                                    Ошибки отлавливал тестер и тестирование это было его задача. Он писал тесты и не только юнит, но и функциональные прогонял. Это же было только для чистоты и простоты кода.
                                      0
                                      Видимо у вас оооооочень длительные итерации, а основное время занято расширением функционала и багфиксингом.
                                        0
                                        Фича релиз каждую неделю. Наверное это долго для аджили.
                                    +1
                                    У нас это используется. Часто помогает избежать простых недочетов, которые программист, который писал код, мог допустить. Я считаю, что применение code review очень оправдано.
                                      –1
                                      Полностью с вами согласен.
                                      0
                                      В Гугле весь код в репозиторий (кроме личных экспериментов разработчиков) попадает только после code review.
                                        0
                                        Хорошая практика, сильно повышает вероятность того, что результаты ревью будут отработаны (а не забыты и на них не забито), и в репозиторий попадет уже хороший код.
                                        0
                                        у нас тоже некоторое время использовался code review
                                        причем не только для новых разработчиков, но и для опытных — молодыми
                                        и это очень сильно подняло качество кода на проекте
                                          0
                                          Когда вы уже Макконнелла прочитаете? То про парное программирование пишут, как про открытие великое, теперь до обзоров добрались.
                                            0
                                            Если чуть развить тему автора, то еще нужны чеклисты, по которым делается ревью и неплохо было бы использовать какой-нибудь инструмент для автоматического кодревью, который бы экономил ваше время и деньги.
                                              0
                                              Скользкий момент тут в том, что самые гадкие ошибки, из серии тут починили, а там отвалилось, отловить это не поможет.
                                              Ну и при достаточно большой и сложной системе один человек это врядли потянет, потому что надо вникать во все тонкости всех частей системы и держать их в голове.

                                              А вообще сама идея неплохая.
                                                0
                                                «Тут починили, там отвалилось» — должны тесты отлавливать.
                                                  0
                                                  Эт понятно, но самые сложные места обычно там, где, тесты прикрутить не удается…
                                                    0
                                                    А это тестеры у нас отлавливали. Они были и реально — очень хорошие. Я так жалею что сейчас в другой компании не он работает.
                                                0
                                                Забейте на этот топик, прочитайте книгу «Совершенный код», там подробно описывается как проводить инспекции кода, а так же про парное программирование в совершенно другой главе.
                                                  0
                                                  вы не поверите, читал с год назад — хорошая книга, примерно по ней и вводили ревьюинг.
                                                    0
                                                    Почему не поверю, очень даже поверю, я даже поверю, что только по ней и вводили — инспекцию кода. Вот только знания уже забылись видать, раз инспекция названа «недопарным программированием».
                                                      0
                                                      ну это я так обозвал… Типа — «Мэр Лужков женщина». В г. Лужки реально мэр женщина. :)
                                                  +1
                                                  Статью нужно было начинать со слов: А вот у нас в компании…
                                                  Потому что первый абзац начисто всех путает. Code Review, который вы описываете, имеет совершненно другие цели нежели Pair Coding.
                                                  А вообще Code Review вполне живая\состоявшаяся практика для оценки кода свежим взглядом. Ревьювер может увидеть какие-либо потенциальные проблемы или ошибки, ну и стандарты кодирования само собой. Времени ревью отнимает совсем не много, а если в команде есть пионеры, за которыми нужен глаз да глаз, так и вообще must have. Вам бы еще CheckStype/PMD plugins начать использовать, у ревьювера еще меньше времени это будет отнимать.
                                                  Ну и насчет парного, одно другому не мешает и вы вполне можете в дополнение к ревью еще и парное программирование использовать.
                                                    0
                                                    спасибо попробуем как-нить совместить
                                                      0
                                                      простите — «Checkstyle»
                                                      +1
                                                      CodeReview который здесь описан является предпосылкой к PairProgramming иначе, PairProgramming может быть поражден из хорошего и долго практикования CodeReview. Тем ни менее цели разные.
                                                      CodeReview ставит своей целью inspeciton: codestyle inspection, design inspection, algoritm inspection и т. д. Фактически это способ удержания качества самого кода, но не качества продукта. У CodeReview есть недостаток, который упоминался выше: это отвлечение вытягивание ревьювещего программиста из «потока», т. е. когда программист занят своей задачей и тут его отвлекают на ревью и ему срочно нужно выгружать, а потом загружать свою проблему из головы и обратно. Есть вариация OfflineCodeReview — реализуется с помощью специальных тулзов и нотификаций — это когда ревьюверу позволено делать ревью в удобное время в режиме оффлайн. Но у такого способа свой недостаток: если у вас достаточно короткие итерации, то вы будете терять ритм из-за длинных циклов попадания кода в основную ветвь разработки.
                                                      Логическим продолжением CodeReview является PairProgramming. Это такая мысль: а почему бы нам постоянно, в каждый момент не ревьювить код. В этом случае нет проблем с выгрузкой/загрузкой проблем в/из головы, потому как оба программиста делают это одновременно. Но PairProgramming ставит своей целью не inspection, а problem solving. И действительно это убийство сразу N зайцев: повышение эффективности — программисты постоянно поддерживают друг друга в потоке не давая отвлекаться на долго и давая друг другу передохнуть, постоянный inspection (review), и значительно улучшенный обмен знаниями и информацией в команде (хороший PairProgramming подразумевает смену пар). Однако часто можно услышать о не работоспособности PairProgramming, основной причиной к этому я считаю психологический фактор программистов и тут как раз важнейшую роль будет играть способность лидера команды к организации общего пространства мышления, но это уже совсем не в ключе этого поста…
                                                        0
                                                        Для всех английских терминов из вашего коммента есть вполне адекватные русские переводы. Problem solving — это зачет. Следующее предложение должно было выглядеть так: «И actually это killing сразу N rabbits».

                                                        ЗЫ. Извиняюсь за придирки — наболело.
                                                          +1
                                                          Прошу прощения, что невольно наступил на Вашу мазоль.
                                                          Дело в привычке, Все слова написанные на английском языке имеют для меня смысл терминов (так например слово «кролик» в контексте обсуждения является метафорой, но не термином). Все термины англоязычного происхождения и заниматься трудностями перевода мне не хочется, поэтому я употребляю их как есть. К том же, так проще выделять термины из текста.
                                                          Еще раз приношу свои извинения.

                                                          P. S. По количеству взаимно наступленых мазолей мы квиты :)
                                                        +1
                                                        То, что вы это делаете, уже лучше, чем если бы нет.

                                                        Но лучше не тикет после коммита создавать, а не коммитить до получения положительного отзыва от ревьюера. Иначе в HEAD trunk'а попадает неотревьюенный код.

                                                        Почитайте про Mondrian — программу, используемую для этого в Google (созданную Гвидо ван Россумом). Например, вот краткое введение, вот видео тек-толка, слайды.

                                                        Сейчас доступен Rietveld — опенсорсовый клон Mondrian'a на App Engine. Исходники на Google Code. Описание.
                                                          0
                                                          > Но лучше не тикет после коммита создавать, а не коммитить до получения положительного отзыва от ревьюера. Иначе в HEAD trunk'а попадает неотревьюенный код.

                                                          Лучше не использовать svn, а перейти на другой инструмент, в котором ветки создаются и удаляются «на раз».

                                                          Мы работаем с git и каждый тикет у нас ведётся в отдельной ветке. Сливает в master как раз человек, который просматривает код.
                                                          0
                                                          cvsspam и SVN::Notify с рассылкой по всему списку разработчиков.

                                                          Вольно или енвольно, мы все становимся участниками Code Review :)

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

                                                          Самое читаемое