Re[5]: Code review (просмотр кода)
От: Bj777x  
Дата: 26.04.19 08:54
Оценка: +4
RF>Зачем нужен рефакторинг? Я всю жизнь без всякого рефакторинга программировал.

Значит это всего лишь следующее: вы плохой программист и ваши программы на самом деле никому не нужны
Re[5]: Code review (просмотр кода)
От: so5team https://stiffstream.com
Дата: 26.04.19 08:56
Оценка: +2
Здравствуйте, RussianFellow, Вы писали:

RF>Зачем нужен рефакторинг? Я всю жизнь без всякого рефакторинга программировал.


Возможно, это потому, что ваши руководители оказывались еще менее компетентными программистами, чем вы. Ибо в команде у более-менее вменяемого руководителя вы бы рефакторингом подобных творений
Автор: RussianFellow
Дата: 11.04 09:34
занимались бы безостановочно, без выходных и проходных. Начиная с азов: с правил именования сущностей, с декомпозиции данных, с декомпозиции функций и т.д. А у совсем невменяемых, таких как я, вы бы и до испытательного срока не дошли.
Re[6]: Code review (просмотр кода)
От: Bj777x  
Дата: 26.04.19 09:03
Оценка:
S>Возможно, это потому, что ваши руководители оказывались еще менее компетентными программистами, чем вы. Ибо в команде у более-менее вменяемого руководителя вы бы рефакторингом подобных творений
Автор: RussianFellow
Дата: 11.04 09:34
занимались бы безостановочно, без


Етить колотить, там только goto не хватает доя полного комплекта
Re[6]: Code review (просмотр кода)
От: anton_t Россия  
Дата: 26.04.19 09:32
Оценка:
Здравствуйте, so5team, Вы писали:

S> подобных творений
Автор: RussianFellow
Дата: 11.04 09:34


Душераздирающее зрелище
Re[8]: Code review (просмотр кода)
От: Sheridan Россия  
Дата: 26.04.19 09:39
Оценка: :))
Здравствуйте, Pzz, Вы писали:

Pzz>Здравствуйте, Vlad_SP, Вы писали:


V_S>>Стиль профессора может быть и не плохой образец для студентов. Однако если цель кодревью — только "исправление стиля", типа исправления расстановки скобочек (Kernighan vs Allman etc.), то это пустая трата времени.


Pzz>Вот чем хорош язык Go, в нем есть один единственный правильный стиль, который жестко енфорсится редактором текстов.

Уроды это придумали. Недоучившаяся школота, не знающая инструментария. Не должен компилятор следить за стилем, для этого отдельные утилиты есть, тот же astyle.
Matrix has you...
Re[7]: Code review (просмотр кода)
От: so5team https://stiffstream.com
Дата: 26.04.19 09:52
Оценка: +1
Здравствуйте, anton_t, Вы писали:

S>> подобных творений
Автор: RussianFellow
Дата: 11.04 09:34


_>Душераздирающее зрелище


"всю жизнь без всякого рефакторинга программировал" (c)
Re[9]: Code review (просмотр кода)
От: Pzz Россия http://pzz.livejournal.com/
Дата: 26.04.19 10:02
Оценка:
Здравствуйте, Sheridan, Вы писали:

Pzz>>Вот чем хорош язык Go, в нем есть один единственный правильный стиль, который жестко енфорсится редактором текстов.

S>Уроды это придумали. Недоучившаяся школота, не знающая инструментария. Не должен компилятор следить за стилем, для этого отдельные утилиты есть, тот же astyle.

Компилятор и не следит, следит отдельная утилита. Компилятору, по большому счету, все равно, а вот любой IDE навязывает стиль, причем один и тот же. Это раздражает первые два дня, а потом понимаешь, что это правильно.

Что до школоты, это вряд ли. Ключевые люди там, в основном, все взрослые и уже отметившиеся в более других общеизвестных проектах (некоторые из этих проектов, впрочем, нонешняя школота может и не упомнить).
Re: Code review (просмотр кода)
От: alpha21264 СССР  
Дата: 26.04.19 10:24
Оценка:
Здравствуйте, RussianFellow, Вы писали:

RF>В чём заключается code review?

RF>Есть ли у вас на работе такое и как часто, каким образом оно проводится? Кто проводит у вас code review?

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

RF>Что бывает в случае плохого результата code review?


Переписываешь так, чтобы было понятно.
Обычно это приводит к переименованию переменных и методов, и написанию комментов.
Сам алгоритм меняется редко.

Очень полезная штука. Особенно если приходится в свой код лезть где-то через полгода.
http://s19.rimg.info/0871fde0709f1bd37b3b012eb22a4583.gif
Течёт вода Кубань-реки куда велят большевики.
Re: Code review (просмотр кода)
От: fk0 Россия  
Дата: 26.04.19 10:27
Оценка: +1
Здравствуйте, RussianFellow, Вы писали:

RF>В чём заключается code review? Есть ли у вас на работе такое и как часто, каким образом оно проводится? Кто проводит у вас code review?

RF>Что бывает в случае плохого результата code review?

Суть code review проводимого в некоторых фирмах, в которых я больше не работаю, заключалась в том, чтоб самому быть на коне, а остальных, соответственно, мокать в дерьмо через закручивание твоих ревью в 20 итераций (в итоге у тебя работа стоит, а у них движется). Когда уже всё просмотрено 19 раз, то на 20-й что-то новое выясняется. В итоге ты свою работу делать не можешь, потому, что ревью будет посмотрено в лучшем случае раз в сутки, и то если менеджер пнет. Выясняется, конечно, что ты, например, точку в комментариях не поставил, что комментарии написаны не так как хотелось бы ревьювяшему, что код отформатирован не так, что названо что-то не так и в таком роде. Т.е. речь не о качестве кода, не о нахождении багов. В той же фирме работали и нормальные люди, которые реально ревьювили и находили ошибки, и не занимались втыканием палок в колеса, так что есть с чем сравнить.

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

Вообще суть code review состоит в том, чтобы:

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

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

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

Зачастую, особенно когда уже есть целая цепочка ревью или отдельная ветка следовало бы как-то документировать работу и я бы советовал писать подробное описание в мерж-коммите (что сделано в целом, в этой ветке, не в деталях, а обощенно, чтоб за деревьями можно было увидеть лес: иначе часто код понятен, а зачем такая масса изменений -- нет). Это имеет отношение к пониманию архитектуры, само архитектурное решение возможно стоит где-то в комментария может, описать на человеческом языке.
Re[6]: Code review (просмотр кода)
От: fk0 Россия  
Дата: 26.04.19 10:58
Оценка:
Здравствуйте, LaptevVV, Вы писали:

LVV>>>Как тогда учить студентов хорошему стилю

RF>>У каждого программиста--свой стиль. Главное, чтобы код был читаемый.
LVV>Нет. Есть общие признаки хорошего стиля.
LVV>Читайте классиков: МакКоннелл, Саттер+Александреску.

С этим трудно поспорить, безусловно общие признаки хорошего стиля есть. Вопрос в их трактовке. И вопрос вообще не технический, не про C++.

Проблема в отсутствии формальных и чётких критериев, что такое хорошо, и что такое плохо. Они субъективны. Вот если бы законы так принимались, это бы работало? Я думаю очевидно, что нет.

И, допустим, рассмотрим работу типичной софтовой фирмы. Где нанято сколько-то программистов и раз в полгода-год действует система награждения хороших и выгоняния плохих (смысл, конечно, не в выгонянии "плохих", реально плохих мало, а в ротации кадров через найм новых более дешёвых сотрудников и в удерживании действительно хороших сотрудников через премирование, например). Это делается так или иначе через KPI реализованный в том или ином виде. Называться оно может какими угодно словами, но в конечном счёте в зачёт пойдут оценки руководителей и/или менеджеров проектов по результатам сделанной работы с их, менеджерской, точки зрения. А с менеджерской точки зрения важны решённые баги, сделанные релизы, достижение поставленных целей. Красивость и качество кода принципиального значения не имеет до тех пор, пока качество кода удовлетворяет с менеджерской точки зрения (проект сделан, багов нет, сроки не затянуты, претензий от заказчика нет).

И вот теперь теперь нехитрая задачка, как оказаться на вершине пирамиды KPI? Для этого совершенно не обязательно выдавать фантастические результаты, более того -- вредно. Проект кончится, что дальше неизвестно. Ну какой автомеханик сделает так, чтоб вы к нему не вернулись ещё раз? Так же и здесь. Но ведь тебя будут сравнивать с другими, выпишут в список, отсортируют по какому-то критерию, голову списка наградят, хвост отрежут. Как оказаться наверху пирамицды, а не в нижнем слое? Кроме очевидной стратегии продираться наверх из последних сил есть более простая: пинками сгонять остальных вниз. Более того, если ты находишься уже близко к вершине или на вершине пирамиды, то выше головы уже не прыгнуть и эта стратегия, сталкивать остальных вниз, остаётся _единственной_. Вроде бы вещь очевидная для тех кто должен уметь управлять людьми.

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

Вот и вся суть грамотного code review. И она ПЕРВИЧНА. Да кого интересует мифическое качество кода? Нормальных людей интересуют регулярные поступления денежных средств в карман. А там -- пирамида описанная выше. То, что в подчинённые нужно вытягивать полных дураков я вообще не от программистов услышал, примерно в таком же ключе рассуждений. Не надо думать, что у программистов будет по-другому.

Конечно, если пирамиду сделать безразмерной, бесконечно высокой, то проблема уходит тоже куда-то в бесконечность. Но как правило пирамидка маленькая и над ней стеклянный потолок. А за ним менеджеры и директора.
Re[2]: Code review (просмотр кода)
От: igor-booch Россия  
Дата: 26.04.19 11:20
Оценка:
Все ли участники команды согласны с тем, что code review приносит выгоду?

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

Не сопровождается ли CR взаимные уступками (я лояльно отношусь к твоему коды, а ты к моему) или, наоборот, местью (ты ко мне придерешься, я к тебе буду придраться).

Есть ли член команды с правом решающего голоса?

Кто обладает правом "вето"?
Отвечайте на это сообщение, только если у Вас хорошее настроение и в Вашем ответе планируются только конструктивные вопросы и замечания
http://rsdn.ru/Info/rules.xml
Отредактировано 26.04.2019 11:46 igor-booch . Предыдущая версия . Еще …
Отредактировано 26.04.2019 11:44 igor-booch . Предыдущая версия .
Отредактировано 26.04.2019 11:22 igor-booch . Предыдущая версия .
Отредактировано 26.04.2019 11:21 igor-booch . Предыдущая версия .
Отредактировано 26.04.2019 11:21 igor-booch . Предыдущая версия .
Re[3]: Code review (просмотр кода)
От: okman Беларусь  
Дата: 26.04.19 13:43
Оценка: 8 (2)
Здравствуйте, igor-booch, Вы писали:

IB>Все ли участники команды согласны с тем, что code review приносит выгоду?


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

IB>Оправдываются трудозатраты на проведение CR и на исправления по результатам CR?


Субъективно:
У нас ревью занимает обычно от получаса до нескольких часов. Иногда вылавливаются довольно тонкие дефекты. Если бы такие
дефекты уходили вместе с версией на тест, их бы обнаруживали в лучшем случае через несколько дней, а то и позже,
когда версия уйдет в релиз. Это не допустимо. Потратить час-два несколько раз в месяц — это достаточно небольшая
плата за CR, я считаю. К тому же есть стажеры и вообще люди, которые в проекте недавно, за ними нужен усиленный
контроль, без CR здесь никуда.

IB>Не бывает ли так, что ради соблюдения, какого-либо принципа проектирования, на котором кто-нибудь настаивает,

IB>код в итоге значительно усложняется, срываются сроки, падает производительность, мотивация, настроение?

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

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


У нас CR — это не стремление любой ценой привести код к 100% идеалу, выбросив без сожаления все, что не
попадает под это определение, а просто некий процесс коллективной оценки кода с целью недопущения явных дефектов и
откровенно плохих практик.

IB>Не сопровождается ли CR взаимные уступками (я лояльно отношусь к твоему коды, а ты к моему) или, наоборот, местью (ты ко мне придерешься, я к тебе буду придраться).


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

IB>Есть ли член команды с правом решающего голоса?

IB>Кто обладает правом "вето"?

В общем случае, замечания участников равноценны. Если возникают споры — вмешиваются начальники отделов и тогда
решаем: "вливать", "не вливать" или "влить, но позже исправить".
Re[6]: Code review (просмотр кода)
От: Lexey Россия  
Дата: 26.04.19 14:35
Оценка: +3 :)
Здравствуйте, so5team, Вы писали:

RF>>Зачем нужен рефакторинг? Я всю жизнь без всякого рефакторинга программировал.


S>Ибо в команде у более-менее вменяемого руководителя вы бы рефакторингом подобных творений
Автор: RussianFellow
Дата: 11.04 09:34
занимались бы безостановочно, без выходных и проходных. Начиная с азов: с правил именования сущностей, с декомпозиции данных, с декомпозиции функций и т.д.


Скорее, такой код никогда не попал бы в репозиторий, так что до рефакторинга просто не дошло бы.

S>А у совсем невменяемых, таких как я, вы бы и до испытательного срока не дошли.


У вполне вменяемых тоже не дошел бы, скорее всего.

P.S. А ТС случаем не жирный тролль? Что-то не верится с существование программиста с таким незамутненным сознанием.
"Будь достоин победы" (c) 8th Wizard's rule.
Re[2]: Code review (просмотр кода)
От: Submitter  
Дата: 26.04.19 14:47
Оценка:
Здравствуйте, CreatorCray, Вы писали:

RF>>Уважаемые коллеги, предлагаю вам обсудить $SUBJECT_NAME

RF>>В чём заключается $SUBJECT_NAME? Есть ли у вас на работе $SUBJECT_NAME и $MORE_QUESTIONS?

На PHP пишешь?

CC>Ну как бы только совсем с тобой не знакомые на такое должны клевать.


+1
Re[7]: Code review (просмотр кода)
От: so5team https://stiffstream.com
Дата: 26.04.19 15:05
Оценка: +1
Здравствуйте, Lexey, Вы писали:

L>P.S. А ТС случаем не жирный тролль?


Были такие подозрения пока на какие-то его вопросы в профильных форумах по C++ не наткнулся.

L>Что-то не верится с существование программиста с таким незамутненным сознанием.


Доводилось видеть отдельных похожих "специалистов". Но в бюджетных или околобюджетных НИИ. Возможно, в каких-то заштатных ВУЗах таких же можно отыскать в штучных экземплярах.
Re[7]: Code review (просмотр кода)
От: LaptevVV Россия  
Дата: 26.04.19 15:08
Оценка:
fk0> Вот и вся суть грамотного code review. И она ПЕРВИЧНА. Да кого интересует мифическое качество кода? Нормальных людей интересуют регулярные поступления денежных средств в карман. А там -- пирамида описанная выше. То, что в подчинённые нужно вытягивать полных дураков я вообще не от программистов услышал, примерно в таком же ключе рассуждений. Не надо думать, что у программистов будет по-другому.
У нас денежный стимул отсутствует.
Поэтому впереди — то самое мифическое качество кода.
И напомню, что есть книжка от Микрософт "Инфраструктура программных проектов".
Тоже неплохие рекомендации прописаны.
А также есть книжка Джуста Виссера "Разработка обслуживаемых программ на языке С#".
И такая же от него по Java.
Там вполне конкретные рекомендации.
Хочешь быть счастливым — будь им!
Без булдырабыз!!!
Re[7]: Code review (просмотр кода)
От: mgu  
Дата: 26.04.19 21:51
Оценка: :)
Здравствуйте, Bj777x, Вы писали:


S>>Возможно, это потому, что ваши руководители оказывались еще менее компетентными программистами, чем вы. Ибо в команде у более-менее вменяемого руководителя вы бы рефакторингом подобных творений
Автор: RussianFellow
Дата: 11.04 09:34
занимались бы безостановочно, без


B>Етить колотить, там только goto не хватает доя полного комплекта


А что плохого в goto? Когда-то написали в газете "Правда", что это плохо, и все повторяют, но никто не может объяснить.
Re[6]: Code review (просмотр кода)
От: mgu  
Дата: 26.04.19 21:53
Оценка: :)))
Здравствуйте, so5team, Вы писали:

S>Возможно, это потому, что ваши руководители оказывались еще менее компетентными программистами, чем вы. Ибо в команде у более-менее вменяемого руководителя вы бы рефакторингом подобных творений
Автор: RussianFellow
Дата: 11.04 09:34
занимались бы безостановочно,


        AfxMessageBox(L"Расчёт параметров схода КО с орбиты\nи времени его баллистического существования завершён");


Так это же rocket science!
Re[6]: Code review (просмотр кода)
От: Эйнсток Файр Мухосранск  
Дата: 26.04.19 23:58
Оценка: :)))
LVV>Я один могу преподавать всю программерскую и математическую часть учебного плана.

А процессор голыми руками в чистом поле из песка изготовить сможете, или нехватает практических навыков?
Отредактировано 27.04.2019 1:39 Эйнсток Файр . Предыдущая версия .
Re[8]: Code review (просмотр кода)
От: LaptevVV Россия  
Дата: 27.04.19 03:59
Оценка:
mgu>А что плохого в goto? Когда-то написали в газете "Правда", что это плохо, и все повторяют, но никто не может объяснить.
Опять же — читайте классиков!
Писал Дейкстра, писал Кнут.
Но статьи старые, в русском инете не найти. Может быть, есть в английском.
Основная мысль состоит в том, что такой код (напичканный goto)
а) трудно читать
б) трудно модифицировать.
Структурное программирование хорошо тем, что каждая конструкция имеет ОДИН вход и ОДИН выход.
Это значительно упрощает как чтение, так и модификацию.
Хочешь быть счастливым — будь им!
Без булдырабыз!!!
Отредактировано 27.04.2019 4:06 LaptevVV . Предыдущая версия .
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.