Удобочитаемость кода, соглашения и стоит ли отпускать

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

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

Теперь один из моих сверстников, у которого есть старшинство, скажем, из-за продолжительности его пребывания там. Умный парень, преданный коду, написал важные статьи. Отводит меня в сторону и говорит:

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

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

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

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

Уточнения:

  • Человек, с которым у меня состоялся этот разговор, не мой менеджер, он мой коллега. Мы на одном уровне. Его предполагаемое старшинство основано на том факте, что он был там дольше, чем я.
  • Соглашение о коде ясно говорит, что я могу изменить код, чтобы сделать его в духе соглашения о коде. Конвенцию написал его непосредственный руководитель.
  • @Fattie - в целом я согласен с тем, что обе версии кода - мусор. Я фанат дяди Боба, Грэди Буча, Кента Бэка, Мартина Фаулера и т. д. Так что я знаю, на что вы намекаете. Сейчас я не стал доводить дело до неузнаваемости, так как боялся просто влезть в этот территориальный спор. Я просто следовал соглашению в этом случае.
  • У меня сложилось впечатление, что некоторые из вас полагают, что быть новичком в этом месте - значит неопытным. Пожалуйста, не думайте так.
  • Для меня удобочитаемость кода является предвестником более легкого обслуживания в будущем. Я знаю, что могу срезать углы и оставить все как есть. В конце концов я собираюсь сменить корабль. Будущим разработчикам придется с этим смириться. Не нужно усложнять себе жизнь на 3 строчки кода. Просто хотел собрать мнения, стоит ли драться. В этом отношении (в данном случае) ответ @AO имеет наибольший смысл.
Комментарии не для расширенного обсуждения; этот разговор был перемещен в чат .
Глядя на код в старой редакции (и говоря как программист): я думаю, что разделение длинных сложных строк на несколько строк — это хорошее изменение, оно значительно облегчает чтение. Та часть, где вы вводите %оператора, более спорна. Возможно, старый код был проще для понимания, особенно для младших разработчиков, которые могут не часто использовать этот оператор (за исключением FizzBuzz). Как вы думаете, стал бы он так яростно жаловаться, если бы вы только разделили реплики? Тем не менее, его реакция была очень плохой. Будьте осторожны с этим парнем в будущем...
Я согласен, что оператор мода (%) добавляет хитрости. Но и без него, я уверен, реакция была бы такой же.
В целом я нахожу соглашения о кодировании слишком подробными и, как правило, переоцененными. Они служат важным целям: избегают опасных привычек и делают код читабельным, отчасти за счет уменьшения различий в стилях в рамках совместного проекта. Сосредоточение внимания на деталях, выходящих за рамки этих целей, отвлекает и может привести к обратным результатам. Я не хочу быть злым, но: я часто подозреваю, что люди, которые сосредотачиваются на деталях стиля кодирования, пользуются властью, предоставленной правилами, чтобы вмешиваться туда, где они иначе не могли бы. Совет: сосредоточьтесь на сути. Это может включать существенные нарушения правил кодирования.
@ Stankar0x, я ответил на аналогичный вопрос несколько лет назад, основываясь на своем опыте более десяти лет назад. Вы можете прочитать это здесь, и, возможно, это будет полезно: Как удержать себя от рефакторинга рабочего, но ужасного кода? Тогда я был на вашем месте и обнаружил, что есть несколько веских причин оставить плохой код в покое (каким бы болезненным он ни был).

Ответы (12)

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

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

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

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

Основываясь на цитате коллеги, между угрозами, запугиванием и преувеличением я очень сомневаюсь, что они из тех людей, которые могут точно говорить за других, фактически учитывая чье-либо мнение.
Re По словам старшего разработчика, его стиль кодирования имеет смысл для команды. Подозреваю, что это не так. Это «его код», к которому этот новичок имел наглость прикоснуться. Я подозреваю, что никто не прикасается к его коду, потому что (а) он совершенно нечитаем, но волшебным образом работает, и (б) парень настолько обидчив о «своем коде», что любой, кто посмеет прикоснуться к нему, получает лекцию.
@DavidHammen: Самый токсичный коллега. Напоминает мне одного старшего коллегу, который был категорически против ревью кода, потому что не хотел, чтобы кто-нибудь проверял его код...
Хотя я в основном согласен с ответом, я не согласен с обоснованием «По словам старшего разработчика, его стиль кодирования имеет смысл для команды». (1) Код каждого имеет смысл для себя. Вы должны проверить мнение других . (2) Старший может применять стандарты кодирования, основываясь на том, что он старший, в некоторой степени независимо от стандарта, который он устанавливает. Тот факт, что ведущий разработчик обращается к OP на личном уровне («зачем вы это делаете?», а не «стандарт кодирования [такой-то]»), предполагает, что ведущий разработчик принимает решения, а не [. .]
[..] в соответствии с установленным руководством, которое объективно определено. Если существует такое установленное руководство, он должен ссылаться на руководство, а не обижаться на действия OP, делать преувеличенные заявления о «100 лет ремонтопригодности», угрожать лишить OP доступа к коду или высокомерно утверждать, что его код (однозначно) не трогать. Если цитата ведущего разработчика в каком-то смысле верна, существует серьезная проблема с самоуверенностью ведущего разработчика и его упреждающим предотвращением всего, что указывает на его честную ошибку (или несовершенное решение).
Тот, кто говорит «это мой код… не трогайте мой код», тут же теряет почти всякое доверие в отношении коммуникативных навыков и работы в команде. Даже если он прав в том, что его образ жизни лучше, это едва ли не худший из возможных способов выразить это. Очевидно, я не знаю всех подробностей, и, возможно, у него просто был плохой день, но такое токсичное взаимодействие со старшим разработчиком является достаточной мотивацией, чтобы начать полировать свое резюме во многих сценариях.

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

Поэтому я бы отреагировал следующим образом:

С: Зачем ты это делаешь? Вы меняете мой код. Мы команда ...

Дж.: Я пытался сделать так, чтобы он соответствовал соглашению о написании кода. В нем говорится, что мы должны сделать XXX и улучшить существующий код, когда увидим его?

А потом слушайте, что он говорит, и искренне старайтесь понять его точку зрения.

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

Что мне делать, если я увижу в вашем коде что-то, чего не понимаю или думаю, что его можно было бы сделать проще?

И снова слушай, чего он хочет.

Это решает несколько задач:

  1. Его довольно эмоциональная реакция указывает на то, что он эмоционально вложен в свой код и чувствует, что его экспертное мнение не уважают. Спрашивая его опыта, вы показываете, что цените его опыт, что должно смягчить дискуссию.
  2. Это говорит о том, что вы не хотели проявлять к нему неуважение или выставлять напоказ социальные нормы. Скорее, это заставляет его понять, что вы просто еще не знали, чего от вас ждут, и что вы пытаетесь это исправить.
  3. Он разъясняет, что от вас ожидается, чтобы избежать дальнейших случайных конфликтов.
«Это разъясняет, что от вас ожидается, чтобы избежать дальнейшего случайного конфликта». Я думаю, что коллега уже ясно дал понять: «Прикоснитесь к моему коду еще раз! Прикоснитесь к моему коду еще раз! Я вызываю вас…» — Так что я не думаю, что участие в предложенном разговоре будет конструктивным.
Умение слушать — один из самых мощных инструментов для завоевания доверия. Иногда мы настолько поглощены тем, чтобы показать кому-то, насколько наш путь лучше, что забываем, что не имеет значения, лучше ли наш путь, если нам никто не доверяет. И время от времени мы можем обнаружить, что наш путь не лучше!
@Fildor: В гневе люди иногда делают опрометчивые заявления, которые после спокойного размышления они признают далеко не идеальными решениями. В этом случае я считаю необходимым пересмотреть, потому что предлагаемое решение не является устойчивым: что должен делать OP, если ему даются задачи, требующие существенных изменений в коде этого старшего?
В общем, именно так я и подхожу к проблемам такого рода. Как бы мне ни нравился ваш ответ, в данном случае его нельзя применить. Раньше я пытался оценить ожидания, но меня отмахивали такими ответами, как: «Ну, соглашение о коде не соблюдается» или «Код был написан давно, его все равно никто не читает. Все в команде это понимают, и так должно быть».

Вы уже отменили изменения. Так что пусть это идет.

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

Это особенно важно, если у вас нет такого большого социального капитала с руководством, как у этого человека.

Кроме того, не забудьте добавить модульные тесты, прежде чем изменять слишком много кода. Если вы внесете ошибки в его код, вы никогда не услышите, как он закончился. И когда ты найдешь что-то в нем, что стоит изменить. Затем продолжайте, но будьте осторожны и следуйте официальным соглашениям (и если официальные соглашения не касаются конкретной проблемы, по крайней мере следуйте соглашениям Code Complete ).

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

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

+20 миллионов за упоминание юнит-тестов. ;D Это сделает любые изменения более безопасными для реализации!
@akaioi Я бы пошел дальше. Не трогайте код, если только это не необходимо для исправления неудачного модульного теста. Не чини то, что не сломано.
@эмори, я не согласен.
@emory, все сообщество Code Review не согласно с этим утверждением.
@Mat'sMug, вы не можете написать ни одного ошибочного модульного теста, но вы все еще хотите возиться с кодом. Либо код идеален как есть, либо вы его не очень понимаете. Продолжайте писать модульные тесты, пока не найдете ошибку, а затем проведите рефакторинг.
@Эмори нет. юнит-тесты проверяют, делает ли код то, что ему нужно, а не читабельно ли он реализован. И если ваши тесты превращают детали реализации в спецификации, то ваши тесты отливают весь код в цементе.
@Mat'sMug, зачем вам читать код, кроме (1) исправления ошибки или (2) добавления новой функции. если вы исправляете ошибку, вам следует начать с написания этого неудачного модульного теста. если вы добавляете новую функцию, отсутствие этой функции является неудачным модульным тестом. если вы читаете код для развлечения, остановитесь и поработайте по-настоящему.
@emory - ОП пытался исправить ошибку. Код был плохо написан и нуждался в рефакторинге, и только тогда ошибка была обнаружена в другом месте (несколько строк ниже отрефакторенной части). Это абсолютно код, который мне нужно было бы реорганизовать, просто чтобы следовать ему, и когда он сталкивается со «старшим» разработчиком, его отношение дает возможность напомнить им, что я бы не исправлял ошибку, если бы они не оставили ее там и не пришлось бы проводить рефакторинг, если бы он соответствовал корпоративным стандартам. Единственный «выход», который я бы дал им, — это если этот раздел кодовой базы предшествует тем (что вполне может быть).

Многое происходит

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

Исправления ошибок и стилей

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

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

Разные взгляды

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

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

Жесткий код

Код был труден для понимания с самого начала.

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

Атака

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

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

Его код

Он не хочет, чтобы вы редактировали его код.

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

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

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

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

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

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

В заключение

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

+1 за код Hobbyist - многие старые кодеры пришли из школы Hobbyist/RockStar, согласно которой «ошибки делают только другие люди», и требуется много саморефлексии и опыта, чтобы понять, почему нам всем нужны другие аспекты программного обеспечения. такие как модульное тестирование и удобочитаемость.
Я ценю решение проблемы, как это сделали вы. Обычно я использую метод Мартина Фаулера и рефакторинг кода по ходу дела, чтобы сделать его более понятным или упростить. Таким образом, изменения стиля и исправления ошибок, а также сложный код — все это решается им. Это здорово, когда у вас есть модульные тесты для подтверждения ваших предположений. Но также работает, когда вы выполняете ручное тестирование в погоне за ошибками. Также, когда оригинальные программисты недоступны, или когда люди заняты и не хотят или не имеют времени обсуждать с вами детали.[..]
[..] Вот несколько ссылок для тех из вас, кто интересуется этим: ссылка или его книга: Рефакторинг, улучшение дизайна существующего кода.
@ Stankar0x Интересное чтение (хотя и немного устаревшее). Я не совсем уверен, что вы это имеете в виду, но под заголовком «Ошибки и исправления стиля» я никогда не хотел сказать, что вы не должны этого делать. Я просто хотел сказать, что может быть лучше выполнить refactor-commit-refactor-commit-fix-commit вместо refactor-refactor-fix-commit. Я знаю, что я немного крайний, когда дело доходит до разделения разных вещей в разных коммитах, но я имел в виду, что если бы исправление было на пару строк ниже рефакторинга, это могло бы помочь снизить остроту проблемы. несколько, если они были отдельными коммитами.

Видел ряд ответов, в которых говорилось: «Отпусти». Я не могу с этим согласиться, потому что этот вопрос будет подниматься снова и снова.

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

Вы удалили свой конкретный пример (который, как мне показалось, был слишком плох, он предлагает некоторый контекст), но позвольте мне добавить одну вещь... Эта потенциально сложная часть (увеличение счетчика индекса по модулю для возврата к 0) должна быть понятна для J Random Programmer, но не мешало бы добавить краткий комментарий вверху строки в качестве напоминания... ;D

Я согласен, что конкретный пример был полезен, и он связан с модулем: поскольку он не упростил чтение расчета индекса, он изменил расчет индекса. Это был oldindex+ 1 >= count ? 0 : старый индекс +1; новый это oldIndex +1 % count. значение oldIndex 4 и число 3, old = 5>=3 результат 0. New = 5%3 результат 2. Возможно, этого не происходит, но... код имеет как синтаксис, так и семантику. Изменение семантики влияет на удобочитаемость — оно меняет концепцию, даже если не меняет результат. Исправление граничной ошибки в цикле for не меняет намерения, возможно, переписывание с помощью LINQ.
ИМО в данном случае по модулю менялось не только вычисляемое значение, но и концепция происходящего. Теперь oldIndex является переносимым или циклическим индексом, где 0, 10 и 2 145 690 — одно и то же, одинаково допустимое значение для счета 10.

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

И ваш коллега пытается запугать вас, не заблуждайтесь.

Во- первых , код, над которым вы оба работаете, не принадлежит ни ему, ни вам. Он принадлежит тому, кто платит вам.

Во- вторых , вы не должны делать то, что он говорит, что вы должны делать, если только он не ваш начальник, чего он не делает. Вы должны делать то, что хочет ваш работодатель.

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


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

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

Хотя все пункты действительны, эта ситуация произошла со мной, и я могу посоветовать избегать конфликтов и масштабирования. Люди не логичны, старший парень может пасть как оскорбление «у тебя код хромой, я все изменю так, чтобы ты выглядел жалко», да, люди могут быть такими чувствительными. У руководства всегда полно проблем, и он может подумать: «О, черт возьми, теперь мне нужно тратить свое время на решение конфликтов между двумя большими детьми, и почему новые ребята тратят время на исправление рабочего кода, неважно, если он уродлив». Конечно, многое зависит от вас, коллеги. Если я старший, меня это не волнует.
Конфликты в команде — самый верный способ сделать вашу жизнь невыносимой
@jean, согласен, что конфликты между членами команды — это ужасно, но коллега может создать такую ​​ситуацию. Я думаю, что эту ситуацию лучше всего решить, сделав ее проблемой тимлида, это его/ее команда.

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

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

На мой взгляд, вы улучшили читаемость кода по большинству формальных показателей: условия более логично организованы, появились фигурные скобки и прочее. Однако ваше введение оператора % (целочисленный остаток) — это хитрый трюк, который запутает 90% младших разработчиков и, по-видимому, некоторых старших. Так вот, слишком многословный код — это плохо, избыточные проверки — это плохо, а Умные Трюки — просто ужас, если только вы не уверены, что все разработчики не менее умны, чем вы. И у вас уже есть доказательства, что это не так.

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

+1 за запрос ввода перед внесением изменений. Это может разрядить подобные ситуации еще до того, как они произойдут. Большинство людей с таким сильным чувством владения кодом будут согласны с изменениями, если они смогут оставить свой след или сделать это вместе.
Трюк хорош только тогда, когда он работает. В этом случае использование % вместо условного изменяет поведение. Полученный код может быть неверным. Хитрый правильный код всегда лучше, чем «чистый» неправильный код. Конечно, при осторожности вы можете получить четкий и правильный код.
@Brandin ИМХО, изменений в поведении не было. Исходный код в основном говорил: (если x+1 >= длина, то 0, иначе x+1). Умный трюк состоял в том, чтобы переписать его как ((x+1)%length) . Это точно так же. Основное отличие состоит в том, что Трюк с Кливером не очевиден.

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

Несмотря на то, сколько усилий уходит на разработку универсальных стандартов кодирования/документации, КАЖДОЕ РАБОЧЕ МЕСТО БУДУТ ИМЕТЬ СВОИ СОБСТВЕННЫЕ СТАНДАРТЫ , и требуется время, чтобы понять, что это такое.

Что касается CAPS ... Стандарты явно не являются стандартами, если их нужно «выводить» ... Они являются стандартами, если они четко определены в политических документах. То, что существует только как набор социальной динамики и условностей, передаваемых одним наблюдением, не является стандартом. Никакое рабочее место не может обеспечить это, особенно с агрессивностью рассматриваемого здесь коллеги. Я не думаю, что совет сесть и заткнуться хорош; если у работодателя ОП есть формальные стандарты, он / она имеет право быть сбитым с толку таким противоположным и решительным вознаграждением за соблюдение (при условии, что они правильно его прочитали)
Это просто двойной крик или тройной крик, когда все написано ЗАГЛАВНЫМИ БУКВАМИ И выделено жирным шрифтом ? :)
Если бы он был сломан, кто-нибудь уже починил бы его. - вы явно никогда не работали над надлежащим унаследованным кодом. :)
@underscore_d О, поверь мне, я согласен с каждым твоим словом. Но на практике, я думаю, мы оба знаем, насколько нечеткими могут быть некоторые «стандарты».
@cale_b Нет, это было больше для акцента. См. другие ответы, где заголовки выделены жирным шрифтом или прописными буквами. Я просто делал это в середине предложения, чтобы подчеркнуть основную мысль.

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

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

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

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

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

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

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

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

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

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

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

У вас есть две альтернативы:

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

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

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

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

Существует множество инструментов, которые автоматически форматируют код для вас в соответствии с набором настраиваемых правил. Они не оставляют много места для интерпретации условностей.

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

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