Каков наилучший способ для программиста справиться с проверкой кода?

16

Я довольно новичок в обзоре кода, но я уже много лет занимаюсь программированием, что не всегда делает вас хорошим программистом!

Если рецензент изменит ваш код и построчно проведет его с вами, что вы будете делать, если не согласны? Иногда вы делали выбор X, и рецензент меняет его на Y, и вы думали о Y, но решили не делать этого из-за недостатков, но рецензент утверждает, что эти недостатки не важны. Должны ли вы выражать свое несогласие словами или просто не слушать их?

Это трудно, если вы не умеете принимать критику, и если рецензент является более старшим человеком в организации. Было бы не хорошо выглядеть слишком оборонительным.

Пол Ричардс
источник
1
есть обсуждение, это не обзор, если это только односторонний трафик
NimChimpsky
@gnat: Этот вопрос явно не дубликат. Посмотрите на вершину проголосовавшего, принятого ответа на этот вопрос. Если бы это было дано как ответ здесь, это было бы понижено, потому что это не отвечает на этот вопрос.
Майкл Шоу
@gnat: Другой вопрос о том, как отклонить чужой код в обзоре, а этот вопрос о том, как справляться с критикой собственного кода в обзоре. Единственное сходство, которое я могу видеть, состоит в том, что оба включают обзоры кода.
Майкл Шоу

Ответы:

19

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

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

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

Также помните, что рецензент тоже не идеален. У них может быть идея, что Y - способ сделать это, и они не поняли, что X лучше. Вы должны объяснить причины, по которым вы сделали это, способом X. Рецензент может согласиться с вами или сказать, почему Y - лучшее решение - могут быть и другие факторы, о которых вы не знаете, что он делает.

Короче говоря, отзывы - это способ заставить членов команды сообщить об изменениях своего кода. Так что поговорите с рецензентом об этом.

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

gbjbaanb
источник
7
«Рецензент обязан проверять код на предмет реального несоответствия и дефектов, а не использовать его как средство для написания вашего кода так, как он сделал бы». +1 за указание на это.
Джорджио
+1 «Отзывы - это способ заставить членов команды сообщить об изменениях своего кода»
Квеббл
20

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

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

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

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

Килиан Фот
источник
4

Другие ответы уже содержат очень хорошую информацию. Я хотел бы немного расширить некоторые аспекты, на которые намекал gbjbaanb (см. Мой комментарий к его ответу).

По своему опыту я наблюдал различные виды обратной связи при проверке кода:

  1. «Я искренне верю, что это неправильно / неоптимально, и что вы должны изменить это таким образом». Обычно я серьезно отношусь к такого рода обратной связи, и я буду против предложенного изменения только в том случае, если думаю, что у меня есть сильные стороны против этого.
  2. «Я нахожу ваше решение в порядке, рассмотрите этот вариант, но вам решать, примете ли вы изменение или нет». Я тоже серьезно отношусь к такого рода обратной связи: рецензент предлагает альтернативу, хотя у них нет твердого мнения о том, что их решение лучше. У меня есть возможность чему-то научиться, и я приму изменение, если оно мне понравится больше. В противном случае рецензент считает, что я могу сохранить код по своему вкусу.
  3. «Я бы сделал это по-другому, поэтому вы должны изменить его, точка.» Или даже «О, я изменил ваш код, потому что ...», изменение не было предложено, оно уже зафиксировано! Дается краткое объяснение изменений, с намеком на то, что у нас не так много времени, чтобы обсудить детали, потому что мы должны перейти к следующему заданию.
  4. Рецензент предлагает небольшие изменения тривиального характера, которые не улучшают код, а просто делают его другим. Когда его просят обсудить предлагаемые изменения, рецензент вступает в длительные дискуссии о тривиальных деталях, пока вы не сдадитесь из-за исчерпания.

Варианты 3, 4 могут быть замаскированы под 1 и 2: «Было действительно важно сделать это так, как я предложил». или «Вы можете отказаться от изменений, если вы действительно хотите», - сказал тоном, который на самом деле означает прямо противоположное тому, что говорится. В случае, если вы выступаете против изменений, которые считаете ненужными, владение общим кодом может быть использовано в качестве оправдания такой позиции: «Код принадлежит не вам, а команде»! Вы можете сказать, когда намерение рецензента не является честным, если рецензент не очень открыт для обсуждения, раздражается и пытается навязать свое решение любой ценой. В основном они уже решили, и любое дальнейшее обсуждение - просто трата времени.

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

Варианты 3, 4 предполагают, что идет какая-то мощная игра: важно то, делаем ли мы это по-своему или по-своему, а не то, что мы пытаемся найти хорошее решение, которое удовлетворяет обоим. Это может быть связано с эго рецензента, но также и с тем, что он не способен понять какой-либо код, который не следует их мышлению. Как правило, их беспокоит код, который им не кажется знакомым и который хотел бы навязать свой путь всей базе кода.

Если ситуации 3 и 4 случаются слишком часто, работа в команде может стать довольно неприятной. В такой ситуации я бы подумал об уходе из команды.

Джорджио
источник
Я обнаружил, что, если я когда-либо чувствую, что мне выпадает 3 или 4, я должен либо продемонстрировать, что они делают, на самом деле не работает («Видите, это на самом деле терпит неудачу, если фамилия клиента пуста»), или выписать оба решения, и проверьте, действительно ли мое предлагаемое решение действительно стоит изменений (возможно, мой код более читабелен, но медленнее, или наоборот, в этих случаях я обычно не буду беспокоиться о просмотре изменений, если только разница не является значительной (либо в читабельности или скорости)).
Scragar
@scragar: Правда: всегда стараются придерживаться фактов. Однако иногда это может быть утомительно. Пример (в контексте довольно обширного коммита): вы должны сравнить std :: string с QString. Ваше решение: конвертировать std :: string в QString и использовать метод QString для сравнения. Изменение рецензента: конвертируйте QString в std :: string и используйте метод std :: string для сравнения. Вы можете думать о бесконечных вариациях того, как преобразовать код в эквивалентный код, просто чтобы показать, что вы были там. Очень важно различать конструктивную обратную связь и придирки.
Джорджио
3

Для решения вашей проблемы, что делать, если вы не согласны ...

Говоря это, наш путь, как уже отметили большинство людей.

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

Можем ли мы положить отдельный билет на это?

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

Майкл Даррант
источник
1

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

cseder
источник
1

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

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

Это не должно приводить к изменению кода без обсуждения / понимания решений кодеров во время разработки.

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

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

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

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

Кевин Хогг
источник
0

Похоже, ваш код еще не проверен :-)

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

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

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

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

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

Если это произойдет, подумайте очень, очень усердно: есть ли у вас проблемы с принятием обоснованной критики? Если это так, вам нужно изменить свое отношение. Вы слишком неопытны, чтобы понять, почему рецензент прав? Если это так, это не проблема. Доверяйте рецензенту и учитесь. Вы уверены, что знаете лучше, чем рецензент? Примите обзор, но спросите третьего доверенного разработчика о его мнении. Помните, что вы можете быть действительно уверены в себе и быть правы, но вы также можете быть действительно уверены в себе и ошибаться.

gnasher729
источник