Ненаписанные правила переписывания кода другого члена команды [закрыто]

40

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

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

sglahn
источник
5
Я не вижу никакого вреда в удалении кода, если это необходимо - при условии, что вы используете контроль версий. Другие ответы на этот вопрос достаточно хорошо описывают этику удаления чужого кода.
Аноним
9
Я не думаю, что это конкретно упомянуто ни в одном из [превосходных] ответов: даже если существующий код заслуживает переписывания, если он работает сейчас, очень вероятно, что переписать будет потраченное время (деньги). Я узнал, что во многих сценариях быстрый и грязный путь. Вам платят за написание кода, поэтому вы должны быть максимально эффективными. Вам не нужен хорошо спроектированный фреймворк для того, что будет запущено всего несколько раз. Вы не хотите занимать неделю, чтобы сделать что-то, что можно было бы сделать за день, используя плохие методы проектирования. Много раз менеджеры предпочитают такой подход.
Таз
9
@taz - это основа шаблона Big Ball of Mud ( laputan.org/mud ).
Мэтью Флинн
@MatthewFlynn - Отличная статья. Фраза «Большой шарик грязи» встречалась столько раз, что выглядела как опыт дежавю в «Практике» Аллана Айверсона ( youtube.com/watch?v=eGDBR2L5kzI ). ;-)
Happy Green Kid Naps
3
Нужно больше контекста. Почему вы меняете код, какова область изменения - часы, дни, недели, месяцы работы. Кто платит за изменения и действительно ли это нужно. Кто одобряет необходимость изменений. Что вы обрабатываете и как это так плохо провалилось, что вы попали в этот беспорядок с самого начала.
Mattnz

Ответы:

62

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

Вход и переписывание без предварительного общения - рецепт злой воли.

Мэтью Флинн
источник
1
Да, я узнал из опыта, что некоторые разработчики могут быть очень взволнованы, если вы даже исправляете ошибки в их коде, не спрашивая. Конечно, это было в проекте без отслеживания проблем или планирования задач, и я удивлен, что это когда-либо даже отправлялось.
пушистый
+1 за «общение с разработчиком» - у нас есть ошибка (ну, по крайней мере, одна, вероятно, больше), которую теперь ожидает заказчик ... И так далеко похоронен, что я был единственным, кто смутно знал, почему он остался в одиночестве.
Изката
3
Я не согласен с «хорошим общением» в принципе, но я думаю, что этот ответ немного банален. Отбрасывая все хлопотные вопросы о том, какие обстоятельства привели к тому, что этот вопрос задавался, на самом деле не должно иметь ни малейшего значения, отвечает ли разработчик «да» или «нет» на вопрос «должен ли я переписать ваш код?» потому что это не обязательно будет ответ, который лучше всего подходит для команды или продукта. Конечно, нужно попытаться выяснить все, что можно, об исходных предположениях и решениях, но как мы узнаем, что ФП еще не сделал этого?
Aaronaught
2
@Aaronaught - Вопрос о том, стоит ли сначала спрашивать оригинального разработчика, подразумевает, что ОП еще не разговаривал с оригинальным разработчиком и, следовательно, не пытался обнаружить все, что он может. Я надеюсь, что ответ противоположен банальному - это фундаментально.
Мэтью Флинн
1
Если вы на самом деле практикуете коллективное владение кодом, то у вас есть право изменить, переписать или удалить любой код, если вы сочтете это целесообразным. Это особенно верно, если вы занимаетесь парным программированием. Теперь, сказав, что перед серьезным изменением (например, переписыванием) кода, который написал какой-то конкретный человек, было бы разумно поговорить об этом с ними. В большинстве случаев, которые я видел (в том числе и со своим собственным кодом), они отвечали: «О да, извините! Этот код - катастрофа; я просто не сделал это правильно. Вот несколько идей о том, как его улучшить» Пожалуйста, беги с этим! "
Джефф Григг
35

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

  1. Вы абсолютно уверены, что говорите о переписывании, а не о рефакторинге? По определению, изменения стиля или структуры кода, которые приводят к улучшенной (или просто другой) реализации без изменений внешнего поведения, являются рефакторингом, а не переписыванием. Разбиение монолитной подпрограммы из 500 строк на набор из 50 подпрограмм может потребовать написания довольно большого количества нового кода, но это не переписывание. Термин « переписать» подразумевает выбрасывание всего продукта или функции и начало с нуля; это не то, что нужно воспринимать легкомысленно.

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

  3. Знает ли менеджер / технический руководитель о проблеме, и если нет, то почему? Независимо от того, какой у вас процесс, за качество кода обычно отвечает менеджер по разработке. Он или она - первый человек, о котором вы должны спросить - очевидно, не в контексте «такого-то написанного дерьмового кода», а просто «Я думаю, что вижу здесь серьезную проблему, мы можем поговорить о переписывании?» Если это не гарантирует такого рода обсуждения, то, может быть, это и не требует переписывания?

  4. Если размер и область действия нарушающего кода достаточно велики для серьезного обсуждения переписывания, почему он принадлежит исключительно одному разработчику? В большинстве случаев, если не во всех случаях, когда я видел код и думал, что «переписать», его затоптали десятки разных людей, и эта плохость является прямым результатом накопления несоответствий стилей, ошибочных или измененных предположений и старого доброго модные хаки Код со временем теряет шипы именно благодаря совместной собственности.

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

  5. Вопрос гласит, в первом абзаце any developer can change any line of code to add functionality, to refactor, fix bugs or improve designs. Переписать не будут делать эти вещи? Или это сделает что-то дополнительное - и если да, то что? По сути, то , что есть ваши причины для желающих сделать рерайт, и как вы уверены, что они будут приносить пользу продукта или команды? Вам не нужно отвечать за меня, но вам лучше иметь несколько объективных замечаний, если и когда вы решите обсудить это с командой.

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

Aaronaught
источник
3
+1 Единственно правильный ответ в этой дискуссии.
Mattnz
Мой опыт работы с командами XP с коллективным владением кодом (и парным программированием) заключается в том, что серьезные изменения в дизайне или «переписывание» частей системы обычно включают в себя обсуждение с большинством команды (всех, кого это волнует). С критической функциональностью ядра существующий дизайн - лучшее, что команда могла сделать в то время. Если это не может быть легко изменено, полезно узнать мнение каждого о том, что, по его мнению, должно быть сделано, и о различных способах его улучшения. Если вы не можете достичь консенсуса, попробуйте некоторые «спайки» или эксперименты для сбора информации.
Джефф Григг
11

Почему вы редактируете код?

Есть ли ошибки или это более фундаментальный недостаток дизайна?

В первом случае я бы беспокоился о переписывании, чтобы исправить мелкие ошибки в коде.

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

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

ChrisF
источник
3
Пока это единственный ответ, с которым я могу согласиться. Если что-то нужно переписать, я переписываю это. Я даже не смотрю, кто был первоначальным автором; почему я должен? Предполагая , что существуют тесты (там находятся тесты, не так ли?), И при условии , что его предназначение достаточно ясно , или объяснить комментариями (если нет, то это , безусловно , должно пойти), то я довольно быстро , если я сломал что - нибудь знать. Конечно, я говорю о переписывании нескольких классов, а не целой среды, но если перезапись настолько велика, то это скорее проблема планирования / планирования проекта, чем проблема совместного владения.
Aaronaught
6

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

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

Мэтт С
источник
5

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

  • Это занимает значительное количество времени. Если кто-то еще работает над связанным изменением, вы не хотите тратить свое время.
  • У других людей другая точка зрения. Даже если вы программируете 20 лет, кто-то другой может знать о взаимодействии с кодом, к которому вы редко обращаетесь.
  • Другие разработчики являются «заказчиками» исходного кода. Исходный код написан для чтения другими разработчиками. У них могут быть требования к архитектуре, входные данные о стиле или запросы функций, которые они хотели в этом коде, но не успели. Вы не хотите завершать рефакторинг только для того, чтобы узнать, нуждается ли другой разработчик в рефакторинге другим способом.
Карл Билефельдт
источник
4

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

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

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

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

superM
источник
2
«Я предлагаю вам удалить код через некоторое время» - где он должен оставаться в то же время? В комментируемом блоке? Вот для чего нужен контроль исходного кода, чтобы сохранить резервную копию всего измененного / удаленного кода для удобного поиска, сохраняя при этом текущий исходный код в чистоте.
dj18
@ dj18, я обычно некоторое время комментирую код, чтобы сделать его более наглядным, чтобы каждый, кто имеет дело с кодом, сразу увидел, что он был изменен. Кроме того, так проще искать, когда код только что отредактирован
superM
1
Это еще одна функция правильной системы контроля версий: вы можете просматривать, когда были внесены изменения, сравнивать версии, чтобы точно увидеть, что изменилось от одной регистрации к другой, и связать комментарии с проверками, чтобы объяснить, почему была сделана модификация. Если для других так важно сразу знать, что часть кода была изменена, возможно, отправьте уведомление по электронной почте, а не ждите, пока другие разработчики дадут вам шанс получить ваши комментарии.
dj18
2
Знание, которое требует этот ответ, должно быть получено от первоначального разработчика, должно быть в коде или, по крайней мере, в сопроводительных комментариях или документации. Если это важная информация, то она не должна передаваться кому-то в голову, и политика развития не должна поощрять это. Кроме того, почему бы не удалить старый код? Вы можете получить его обратно, если вам действительно нужно; это то, что контроль версий является для .
Aaronaught
1
@Aaronaught: Я думаю, что SuperM хотел сказать, что-то вроде "здесь быть драконами", или "извлеченные уроки". Хотя опытные программисты должны быть довольно очевидны, тонкие варианты менее очевидны и не могут быть хорошо документированы. (Я согласен, что это плохие признаки, которые необходимо устранить.)
Руонг
2

Какова область этого?

  • Если вы переделываете несколько строк, чтобы сделать их более эффективными, просто сделайте это. Может быть, спросить, есть ли опасность того, что вы стираете тонкое поведение. Сообщите об изменении во время схватки или по электронной почте, как только это будет сделано, если только это не тривиально (исправление опечатки).

  • Если вы переделываете класс приличного размера, вам, вероятно, следует попросить убедиться, что вы понимаете дизайн / поведение. Дайте людям знать заранее, чтобы каждый, кто работает в той же области, мог знать и координировать с вами.

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

Telastyn
источник
1

Если там есть ваши авторы, свяжитесь с ними. Не ТОЛЬКО для этикета, но для дополнительной информации, если есть случаи или обстоятельства, о которых вы не знаете. Иногда они скажут вам что-нибудь ценное.

У нас ужасный контроль над кодом. В настоящее время у меня есть куча вещей, которые требуют проверки и обслуживания, и даже у меня нет никого, с кем можно было бы проверять код. Предыдущие авторы (кроме moi) не удосужились прокомментировать какой-либо код. И они ушли из компании. Там нет никого, чтобы спросить о коде.

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

Дополнительные комментарии делаются в заголовке пакета (как правило, обычно), указывая, какие функции / procs / SQL / etc. были изменены, с датой. Это облегчает поиск разделов, которые были изменены, и эти разделы имеют полную документацию.

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

Марк
источник
1

Из моих наблюдений общая практика такова: никогда не удаляйте код. Просто закомментируйте это и сохраните.

Моя практика - комментировать его при замене. (В основном для справки.) Затем удалите его при окончательной фиксации рабочего изменения. (Это требует контроля версий и понимания того, как отменить изменения.)

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

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

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

Такие теги, как FixMe, ToDo, Bug и Hack, могут использоваться для обозначения кода, который можно изменить позже. (Допустимо помечать ограничения как ошибки и не исправлять их, если они не будут срабатывать при требуемых условиях.) Возможно, это будет ошибкой, если программа домашнего учета переполнена на 20 миллионов долларов, но я бы не стал тратить много времени на исправление Это.

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

BillThor
источник
2
Почти опроверг это, пока я не увидел «Затем удалите его на последнем коммите».
Джошуа Дрейк
1

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

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

С другой стороны, бывают случаи, когда требуется перезапись:

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

  • Новое требование к функции меняет ситуацию так, что целевое мини-переписывание подходит.

В этих случаях я бы не стал разговаривать.

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

Шон Рейли
источник
1

Я думаю, что @aaronaught сделал несколько хороших замечаний, которые действительно приводят к ответу, который я хотел дать, а именно, что это действительно зависит от того, кто вносит изменения (и почему) и кто написал код.

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

В среде Team dev вы не должны (и, возможно, не сможете) общаться с оригинальным кодером, все должно быть ясно из кода.

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

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

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

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

sibaz
источник
-2

Часто код пишется определенным образом по причине. Сообщение изменения автору всегда работает к лучшему.

Чак Конвей
источник