Должен ли я следовать плохому стилю кодирования, чтобы просто следовать установленным правилам на своем рабочем месте?

102

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

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

Должен ли я просто хранить все в огромной функции в соответствии со стилем существующих функций или я должен инкапсулировать аварийные сигналы в их собственные функции?

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

В итоге я сравниваю

showAlarms(){
    // tons of code
}

против

showAlarms(){
   alarm1();
   alarm2();
   return;
}

alarm1(){
    ...
    printf(...);
    return;
}

РЕДАКТИРОВАТЬ: Спасибо за совет всем, я решил, что я собираюсь разработать свой код с учетом фактора, а затем спросить, что они хотят, и если они хотят все это в одном, я могу просто вырезать из своего факторизованного кода и превратить его обратно в 1 большая функция. Это должно позволить мне написать и протестировать его легче, даже если они хотят всего кода в одном определении.

ОБНОВЛЕНИЕ: В итоге они были довольны факторизованным кодом, и несколько человек поблагодарили меня за создание этого прецедента.

Джастин
источник
117
Весьма сомнительно, что компания, в которой вы работаете, считает, что ваши функции должны быть длиной в 300 строк, потому что «мы всегда так поступали». Это не «соглашения о стиле кодирования»; это просто плохой стиль.
Роберт Харви
64
Это то, что вы должны обсуждать с другими членами вашей команды, чтобы выяснить, какие практики вы видите, являются соглашениями, которые они считают важными для всех, и какие вещи кто-то сделал однажды, а вы - нет. нужно подражать. Без сомнения будет некоторое количество обоих.
Serv
17
@beginnerBinx Все сводится к тому, как вы это сформулируете. Не формулируйте это так: «X делает что-то действительно глупое, я тоже должен делать эту глупость». но скорее как что-то вроде: «Я замечаю, что X делает это с одной стороны, есть ли какая-то особая причина, по которой он делает это так, как он делает; было бы проблемой, если бы я не делал то же самое при написании Y?» Затем он принимает решение о том, стоит ли разбивать старый код или объяснять, почему они должны были сделать это таким образом (неохотно или с энтузиазмом).
Serv
11
О хорошем или плохом стиле часто спорят и редко соглашаются. Обучение в университете заключается в том, что функции должны быть короткими, но если это затеняет смысл, не делайте этого. Тест должен быть ясным, а не бездумно следовать ряду правил.
quick_now
9
Случайные люди в интернете не могут прочесть ваших товарищей по команде, поэтому все ответы, которые вы получаете здесь, являются лишь личными предпочтениями людей. Вы должны поговорить с вашей командой. Является ли длинная функция продуманным принципом дизайна? Они хотели бы, чтобы вы продолжили стиль длинных функций, или они хотели бы, чтобы вы написали то, что вы считаете самым чистым?
JacquesB

Ответы:

102

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

Лично я столкнулся с этим на своем первом набеге на критически важное кодирование в реальном времени. Я видел такой код:

lockMutex(&mutex);
int rval;
if (...)
{
    ...
    rval = foo();
}
else
{
    ...
    rval = bar();
}
unlockMutex(&mutex);
return rval;

Будучи ярким и блестящим разработчиком OO C ++, я сразу же обратил их внимание на риски ошибок, которые они имели, вручную блокируя и разблокируя мьютексы, а не используя RAII . Я настоял, чтобы это было лучше:

MutexLocker lock(mutex);
if (...)
{
    ...
    return foo();
}
else
{
    ...
    return bar();
}

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

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

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

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

Корт Аммон
источник
60
Возможно, более опасной частью примера мьютекса является очевидное отсутствие комментариев к коду, объясняющих, почему он не может использовать RAII. Конечно, если эта явная блокировка / разблокировка присутствует во всей кодовой базе, было бы неудобно добавлять комментарий для каждого экземпляра. В этом случае, возможно, следует написать учебник для начинающих, с которым необходимо ознакомиться новым разработчикам.
Кельвин
10
Конечно, общение со старшими всегда хорошо, и рефакторинг должен проводиться с осторожностью (и с тестированием). И, конечно же, параллелизм / мьютексы - это особые животные. Но при просмотре функций с> 300 строками я бы не стал тратить слишком много времени на поиск «скрытой технической причины», почему эти функции слишком велики. Причина, по которой функции становятся слишком длинными, всегда одна и та же, и они не являются техническими.
Док Браун
8
@DocBrown То, что они не являются техническими, не означает, что они не являются причинами. Разработчикам важно помнить, что они являются частью большей машины. (Я не могу сосчитать, сколько раз мне приходилось переучивать этот урок)
Cort Ammon
4
@DocBrown В любом случае, с старшими разработчиками следует разговаривать. Я выбрал пример, который сделал, потому что легко найти бесчисленные аргументы, почему глупый код глуп. Найти аргументы, почему код, который выглядит глупо, на самом деле не так глупо, сложнее.
Cort Ammon
3
@DocBrown Из ваших комментариев и ответов я думаю, что могу сказать, что разница в нашем мнении заключается в том, что вы исходите из предположения, что код уже «плохой», основываясь на том, какие доказательства вам были предоставлены, хотя я бы предпочел возьмите пути, которые исследуют, является ли код "плохим" сначала.
Корт Аммон
84

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

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

Приложение, из-за комментариев: «300 строк» ​​и «сложно использовать» - ИМХО сильные признаки плохого кода. По моему опыту, очень маловероятно, что существует какая-то «скрытая техническая причина», почему такой код не может быть реализован в более читабельной форме, сравнимой с примером ответа Корта Аммона. Тем не менее, я согласен с Кортом: вам следует обсудить это с ответственными лицами в команде - не для того, чтобы найти неясные причины, по которым код или этот «стиль» не может быть изменен, а чтобы выяснить, как можно безопасно реорганизовать код, не нарушая его. ,

Док Браун
источник
127
Даже компетентные программисты совершают эти преступления в сжатые сроки.
садовник
13
@gardenhead, я могу понять, что рефакторинг 300-строчной функции не позволяет сэкономить время, но невозможно написать 300-строчную функцию, чтобы сэкономить ваше время.
Dangph
39
@gardenhead: когда я обращаюсь к хирургу, потому что мне нужна срочная помощь, я все еще ожидаю, что он уделит время, чтобы вымыть руки, прежде чем он начнет выполнять мне. Это то, чему многие люди должны научиться в нашем бизнесе, чтобы достичь реальной компетенции: всегда оставлять код в лучшем состоянии, чем было раньше, и это также улучшит способность придерживаться сроков. 300 линий функций обычно растут день ото дня людьми, которым все равно, и они всегда будут использовать «крайний срок» в качестве оправдания.
Док Браун
17
@Dangph экономит время, когда вы добавляете в функцию только 5 строк вместо рефакторинга. Они обычно растут, когда начальная функция длинная (более 30 строк), и следующий программист думает: «Это не мой код, я не буду рефакторировать, чтобы не ломать его, просто добавлю несколько строк здесь и там».
Джурис
7
@ Юрис, насколько я понимаю, вопрос заключается в создании новой функции, а не в рефакторинге существующих. Если вы пишете новую функцию, вы не сэкономите время, превратив ее в одну функцию монстра.
Dangph
42

Нет.

В книге Pragmatic Programmer автор рассказывает о теории разбитого окна .

Эта теория гласит:

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

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

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

И ответ нет.

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

И однажды код рухнет.

Раздайте всем по одному экземпляру Чистого кода и Чистого кодера .

И пока вы находитесь в теме, копия TDD также является хорошей.

linuxunil
источник
6
Что если кодовая база - это оранжерея, в которой нет только разбитых окон?
Алан Шутко
8
@AlanShutko Тогда убедитесь, что ваше резюме актуально? Найти другого работодателя сейчас .
Дэвид Монтгомери
12
Код редко "рушится". Все труднее и сложнее добавлять новые функции, на это уходит все больше и больше времени, и смета на очистку кода возрастет, а значит, становится все труднее получить бюджет времени для необходимой очистки.
Док Браун
8
Ух ты, какой произвольной аналогией кажется теория "разбитого окна".
Самтье
4
TDD не имеет к этому никакого отношения. Собираетесь ли вы использовать драйверы Business / Domain / Test / Nothingness, которые ничего не меняют, следуя основам чистого кода. Извините, но действительно утомительно видеть, что «TDD» цитируется везде, где это даже не в теме
Walfrat
25

Да, пойти на это. Структура! = Стиль

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

Быть осторожен

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

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

Быть нежным

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

Джон Ву
источник
«Попробуйте придерживаться потока, который будет иметь смысл для людей, которые привыкли к старому стилю». - Итак, ваш совет программировать так же, как любой некомпетентный клоун делал раньше? Добавление функции 300 строк не поможет.
BЈовић
11
Я не говорил придерживаться потока, который похож. Я сказал придерживаться потока, который они поймут.
Джон Ву
2
Хороший совет. Когда в этом ответе я произвел рефакторинг одной команды на 5 функций , я слегка изменил семантику, предварительно вычислив значения внутри логического выражения, которые изначально рассчитывались только условно. Рецензенты поймали это хотя ;-). Серьезный совет - внимательно изучить и протестировать. Каждое изменение может привести к ошибкам, и это может быть основной причиной, по которой их никто не делал.
Питер А. Шнайдер
6

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

Бернард
источник
1
« Там нет причин, это просто наша политика. »
5

Ответ в обзоре кода.

Реальный вопрос в том, что если вы включите хорошо продуманный код, будете ли вы единственным, кто будет работать с ним?

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

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

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

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

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

candied_orange
источник
5
Если для того, чтобы убедить разработчиков в том, что метод с 300 строками слишком длинный, нужно столько общения и многократных встреч, то я не думаю, что разговор поможет. Проблема с вышеуказанным подходом состоит в том, что если вы запрашиваете разрешение на написание хорошего кода, вы защищаетесь. Но если вы просто напишите хороший код и потратите время на рефакторинг существующего кода, то любой, кто возражает, будет защищен, чтобы объяснить, почему метод с 300 строками хорош, и оправдать тратить время на его возвращение.
Брэндон
3
Я могу вам сказать, что, если бы меня пригласили на серию собраний для обсуждения строк кода для каждой функции, я бы нашел способ избежать таких собраний. Там нет правильного номера, и вы никогда не заставите людей согласиться. Иногда людям нужно показать лучший путь.
Брэндон
Иногда разделение огромной функции (и более точное именование и добавление комментариев и документации, как я понимаю, является необходимым первым шагом перед сохранением изменений)
JDługosz
@ Брэндон Вы, вероятно, не имеете в виду, что это звучит так, но я слышал, что вы правы, и все остальные могут просто с этим справиться. Пока ваш код работает, не имеет значения, если остальная часть команды не понимает это. Конечно, не стоит тратить время на то, чтобы научить их чему-либо. Вы совершенно счастливы наблюдать, как они продолжают создавать 300 строковых функций, пока вы пишете код по-своему.
candied_orange
1
@CandiedOrange Я, конечно, не имел в виду идти против команды. Я имел в виду, что некоторые темы лучше всего изучать, рассматривая их в действии, но попытка заставить команду принять демократическое решение о стиле не будет продуктивной. Я видел последовательность, использованную в качестве оправдания для написания нечитаемого кода. Результат? Более нечитаемый код. Я мог бы организовать серию встреч, чтобы поговорить об этом, или я мог бы просто исправить это, а затем показать людям результаты.
Брэндон
3

Иногда вам нужно плыть по течению.

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

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

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

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

МЕДА
источник
2
Если вы будете просить разрешения на каждый небольшой рефакторинг, у вас всегда будет неуправляемый код, который опасен для бизнеса. Менеджеры только смотрят на стоимость изменения, но редко смотрят на стоимость не изменения.
Брэндон
5
ОП попросили написать функцию, не модифицирующую сотни строк кода
меда
2
Точно @meda! Я в очень похожей ситуации. Я разрабатываю новые модули для внутренней сети компании и нахожу то, что можно охарактеризовать как «годы безнадежности без ремонта», когда серверное программное обеспечение и библиотеки вообще не обновляются с 2006 года. Я не должен убирать этот беспорядок , но это занимает больше времени для написания кода из-за ограничений. (Представьте себе MySQl 5.2, MySQL 5.0). Я не могу ничего обновить, потому что, вероятно, существующий код не будет работать на более новых версиях из-за устаревшего кода.
roetnig
3
«Если это не сломано, не чини». У меня 20-летняя машина, в которой течет немного масла. Стоит тратить деньги на? Нет. Надежно? Да.
3

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

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

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

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

skipy
источник
Я не рефакторинг текущего кода, я просто пишу новую функцию, не уверен, должен ли я просто следовать «соглашению» и кодировать его в 300+ строчках, или разбить его на логические куски, как обычно, но не было сделано в этой части кода базы.
Джастин
1
есть ли у них правила, гарантирующие, что вы пишете новые методы в 300+ строк? если нет, я бы написал это правильно. но я бы очень постарался, чтобы убедиться, что я проверю их, включая ветви. Я прошел через подобные опыты, и обычно вы их завоевываете. Хитрость заключается в том, чтобы сделать это со временем и наладить отношения.
Skipy
3

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

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

  1. Является ли это осознанное дизайнерское решение поддержанным вашей нынешней командой?
  2. Они хотят, чтобы вы следовали этому стилю?

Есть только один способ узнать. Спросите .

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

JacquesB
источник
1
Я полностью согласен и добавляю: «Может быть, вас уволили или попали в беду за то, что вы не следовали тому, что сделали другие?» Тогда ответ - да! Следуя всем плохим вещам, компания требует, чтобы вы сделали и справились с этим.
Роб
1

Существуют разные уровни «стиля».

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

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

Тогда есть библиотеки и фреймворки, используемые проектом.

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

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

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

Эрик Эйдт
источник