Comments 194
// нужно проверить на то-то,
someFunction();
Минутка рекламы — JetBrains IDE умеют распознавать // TODO:… и составлять из них списки (и напоминать при каждом удобном случае, например, останавливая коммит).
Подозреваю, что похожим трюкам можно научить и более прочие IDE.
В принципе по всем четырем пунктам можно придумать автоматизацию, которая будет перед комитом предупреждать разработчика.
По крайней мере NetBeans это тоже мог (лет 5-6 назад).
На базовом уровне достаточно и grep'а.
Vim подсвечивает ярким цветом TODO и FIXME в комментариях
И Visual Studio так умеет.
Хотя в C# еще удобнее использовать директиву #warning, чтобы предупреждения мозолили глаза
Хотя в C# еще удобнее использовать директиву #warning, чтобы предупреждения мозолили глаза
Спасибо не знал, пойду поковыряю настройки.
Eclipse так на дефолтных настройках делает.
YARD позволяет составить список всех @todo тегов.
Даже для Atom есть расширение для подсветки всех TODO / FIXME
RAD Studio также делает списки из TODO.
Да, спасибо автору. Очень полезно!
UFO just landed and posted this here
Бесполезные комментарии часто минусуют
Рискну предположить, что потому что "спасибо" принято выражать плюсом.
А как человек плюс поставит, если у него карма отрицательная?
Да, именно. Пришел человек на сайт. Читает полезные статьи, реально полезные. А оценивать их может только тот, кто здесь очень много времени или по приглашению. Дак им нахер не нужны такие статьи. Существует много англоязычных сайтов, откуда сюда такие же люди переводят статьи, для таких как я. Я и с положительной кармой не мог не чего делать.
Авторы! Поэтому, если кто то пришел почитать статью вашу, и она реально интересная для развивающегося человека. Он вам плюс не где не поставит. А поставят только те, кто и без вас это знают!
Минусуют второй раз за благодарность. Мне пох, я буду благодарить. Положил я на эту сраную Карму!
Авторы! Поэтому, если кто то пришел почитать статью вашу, и она реально интересная для развивающегося человека. Он вам плюс не где не поставит. А поставят только те, кто и без вас это знают!
Минусуют второй раз за благодарность. Мне пох, я буду благодарить. Положил я на эту сраную Карму!
Отрицательная карма — это наказание для пользователя, а не индульгенция на мусорные комментарии.
А об этом написали уже кучу статей. Что здесь минусуют просто так. Сайт умирает в плане качества людей. И это заминусуют, потому что это правда. И на минус нельзя ответить. Администации фиолетово на это. Они заняты получением дохода, и не могут допустить, чтоб можно было ответить на то, что тебя неоправданно заминусовали. Это не приоритетная задача.
Рейтинг, когда то придуманный для полезности человека стал — фиговым листком, которым можно мерятся как длиной органа.
Благодарность автору в словах считается ненужным комментарием. А обсужение того, что у него там херня написана — плюсуется десятками! Всем спасибо, всем пока!
Автору еще раз спасибо за полезную информацию, не могу не чего, кроме комента оставить автору.
Рейтинг, когда то придуманный для полезности человека стал — фиговым листком, которым можно мерятся как длиной органа.
Благодарность автору в словах считается ненужным комментарием. А обсужение того, что у него там херня написана — плюсуется десятками! Всем спасибо, всем пока!
Автору еще раз спасибо за полезную информацию, не могу не чего, кроме комента оставить автору.
UFO just landed and posted this here
Комментарии отнимают у читателя дефицитный ресурс — время. Поэтому комментарии должны содержать информацию.
Отрицательные комментарии обычно содержат полезную информацию (что именно не так в статье) — потому и плюсуются. Комментарии вида "спасибо" полезной информации не содержат — потому и минусуются. Кстати, комментарии вида "автор — козел, у меня такое вот мнение, но аргументов не будет" тоже обычно активно минусуются, если вы не замечали.
PS и да, я втихую ставлю минусы просто так тем комментариям, с которыми я не согласен. Я это делаю не по злобе — а чтобы не отнимать время других людей своим сумбурным комментарием. Потому что минус за комментарий — это и есть форма выражения несогласия. Но если я вижу вопрос "а за что минусуют-то" — стараюсь на него развернуто ответить.
Вам первый комментарий я не минусовал. А остальным комментариям минусы я поставил, и добавил в карму — за оффтопик. Есть же посты, посвященные карме на хабре и системам с рейтингом вообще — там давно уже это все обсудили и не по одному разу. На этом офтопик я заканчиваю.
Resharper тоже отлично распознает тудушки :)
UFO just landed and posted this here
А самое грустное — когда приходишь на новое место работы, а там в коде есть сразу все эти проблемы, и причем везде. Тратишь неделю на то, чтобы объяснить, почему так нельзя, и не достигаешь успеха, ведь «мы всегда так делали, мы так привыкли».
идешь к тим лиду и убеждаешь его. Если убедил — он волюнтаристски распространит новые правила на всех. Если не убедил — смирись и жди пока сам станешь ТимЛидом и будешь устанавливать правила для кода.
Я в свое время выбрал третий вариант — поскорее свалить из этого ада. ИМХО, самый эффективный способ решения проблемы.
Самое смешное, что обычно бывает вот так: тим лид заставит всех писать единообразно, например, иерархия наследования, венгерская нотация, правильное число отступов на табуляции, и т.д. и т.п. Потом проходит время мода меняется. Теперь ОН ЖЕ заставляет google code rules, а венгерская нотация запрещена. Наследование c++ — не глубже 2, приветствуются только шаблоны. В код глянуть — это тихий ужас и кошмар: смесь старого и нового кода — это выглядит интересно.
И ведь известно, что благими намерениями выстлана дорога в ад.
И ведь известно, что благими намерениями выстлана дорога в ад.
Чем больше пишу код и изучаю — тем больше ощущение сколько всего еще надо выучить… А на код написанный больше полугода всегда какой-то «нетакой» уже)
Лживая документация это вообще международный терроризм (привет OpenSSL!). Или давно устаревшая (привет Botan!). В которой примеры написаны с синтаксическими ошибками (опять ты, OpenSSL).
(Полез в код OpenSSL, выяснить, почему размер IV не совпадает с заявленным, увидел 'return -2', решил не использовать больше ее никогда)
(Полез в код OpenSSL, выяснить, почему размер IV не совпадает с заявленным, увидел 'return -2', решил не использовать больше ее никогда)
По поводу этих 2х примеров:
if (true){
someFunction();
}
// нужно проверить на то-то,
someFunction();
Первый пример не так бестолков как какжется, потому что вместо someFunction() может быть несколько строк
кода и проверка нужна на весь блок, поэтому лучше если бы код выглядел так:
//TODO: Need to check all parameters for null
if (true){
someFunction();
}
А в идеале нужно сразу реализовывать всё на чистовую со всеми необходимыми проверками. И да, старайтесь писать коментарии на английском (кто знает что случится с вашим проектом в будущем и может быть он вырастет до интернационального, и в этом случае ваши коментарии будут абсолютно бесполезны).
if (true){
someFunction();
}
// нужно проверить на то-то,
someFunction();
Первый пример не так бестолков как какжется, потому что вместо someFunction() может быть несколько строк
кода и проверка нужна на весь блок, поэтому лучше если бы код выглядел так:
//TODO: Need to check all parameters for null
if (true){
someFunction();
}
А в идеале нужно сразу реализовывать всё на чистовую со всеми необходимыми проверками. И да, старайтесь писать коментарии на английском (кто знает что случится с вашим проектом в будущем и может быть он вырастет до интернационального, и в этом случае ваши коментарии будут абсолютно бесполезны).
Насчет комментариев это да. Изучая VueJs доставляли комментарии на китайском =)
> кто знает что случится с вашим проектом в будущем
На этот случай есть YAGNI
На этот случай есть YAGNI
Насчёт комментариев на английском — не согласен. Если это опенсорс со множеством разноязычных разработчиков — то да, надо комментарить на английском. Если же проект проприетарный и комментарии пишутся для себя — чтобы через некоторое время можно было без напряга вспомнить, что тут происходит — то лучше писать на своём родном. В особенности — если не владеешь английским на уровне разговорного. Потому как использование неродного языка значительно замедляет процесс, да и отвлекает от основной задачи.
Незнание программистом английского я бы тоже пречислял к серьёзным грехам. Никто не требует Шекспировских сонетов в коментах, поэтому достаточно словарного запаса слов так в 500.
Я вообще не понимаю, как можно умудриться не знать англиский, учитывая что 90% документации к фреймворкам и языкам написаны именно на нем.
Запросто. Чтение на английском и письмо на нем же — это разные навыки.
UFO just landed and posted this here
Одно дело — обобщающий комментарий к коммиту. Другое дело — объясняющий комментарий в коде.
Ну нет никакого смысла писать в комментарии то, что и без того видно в коде.
UFO just landed and posted this here
Вот другой пример:
Или вот:
Можно было бы напрячься и написать всё это по-английски. А потом, спустя пару лет, можно было бы напрячься, прочитать это и понять. Только вот вопрос — зачем так напрягаться, если этот код кроме меня и моих коллег никто не видел и не увидит?
// Стартуем задачу и устанавливаем APN, User ID и password:
// формируем строку в три этапа, так как запрос параметра
// возвращает результат всегда в одном и том-же буфере, что
// не позволяет воспользоваться одной командой sprintf.
Или вот:
// Таймаут не считаем ошибкой, так как модуль не отвечает
// "SEND OK", пока реально не отправит данные на
// сервер. Из-за этого таймаут возникает слишком часто -
// а в результате мы выполняем совершенно не нужную
// переустановку соединения с сервером.
Можно было бы напрячься и написать всё это по-английски. А потом, спустя пару лет, можно было бы напрячься, прочитать это и понять. Только вот вопрос — зачем так напрягаться, если этот код кроме меня и моих коллег никто не видел и не увидит?
Ну вот мне приходилось читать комментарии на польском и на венгерском. И спасибо google translate, что он сам понял, что это венгерский!
зачем так напрягаться, если этот код кроме меня и моих коллег никто не видел и не увидит?Как вариант, чтобы можно было продемонстрировать свой код как портфолио при трудоустройстве в иностранную/международную компанию.
Ещё вариант: чтобы лишний раз не переключать раскладку. И не нужно мне говорить про Punto Switcher — он только мешает, когда пишешь код.
Довелось нам недавно ремонтировать платы от российской телефонной станции M200. Человек, который когда-то писал софт к ним, как Вы и говорите, знал некоторое количество английских слов и старался делать вывод сообщений в лог на английском. На Шекспировские сонеты это точно не было похоже.
Например, во время стартового тестирования работоспособности DSP-процессора в лог могла вывестись вот такая строка:
Нет, это не то, что Вы подумали. Это вовсе не сообщение о том, что тест пройден. Как раз наоборот — это сообщение о сбое проверки. После его вывода АТС уходила в перезагруз. Обратите, кстати говоря, внимание на слово 'reseting'.
Спрашивается — зачем было напрягаться и писать по-английски? Эту АТС покупали иностранцы? Вряд-ли.
Так что всё должно быть в меру. Не нужно впадать в крайности.
Например, во время стартового тестирования работоспособности DSP-процессора в лог могла вывестись вот такая строка:
TEST DTMF64 pass... reseting 020202
Нет, это не то, что Вы подумали. Это вовсе не сообщение о том, что тест пройден. Как раз наоборот — это сообщение о сбое проверки. После его вывода АТС уходила в перезагруз. Обратите, кстати говоря, внимание на слово 'reseting'.
Спрашивается — зачем было напрягаться и писать по-английски? Эту АТС покупали иностранцы? Вряд-ли.
Так что всё должно быть в меру. Не нужно впадать в крайности.
Но можно ведь без if:
/* TODO: need to check this and that and also that */ {
someFunction();
}
А является ли болезнь криминалом
Если записана в уголовный кодекс — да. История знает примеры. Вплоть до летальных.
а пример привести
Ну как пример относительно недавних событий — гомосексуализм, считавшийся болезнью, в Германии в 30-40 годах прошлого века.
Гоомсексуализм считался в нацистской Германии развратом, а не болезнью
И болезнью тоже.
Ссылку можно на статью —
Гомосексуализм считался болезнью в Англии и принудительно лечилось (например, лечили Тьюринга), но уголовного преследования не было
Болезнь не считается преступлением по уголовным кодексам — никто не виноват угловно, что он заболел
Все-таки уголовное преступление есть посягание на чужую собственность и чужую личность, а болезнь дело личное, но вредные для общества болезни подлежат различным мерам ограничения, но не уголовного характера
Гомосексуализм считался болезнью в Англии и принудительно лечилось (например, лечили Тьюринга), но уголовного преследования не было
Болезнь не считается преступлением по уголовным кодексам — никто не виноват угловно, что он заболел
Все-таки уголовное преступление есть посягание на чужую собственность и чужую личность, а болезнь дело личное, но вредные для общества болезни подлежат различным мерам ограничения, но не уголовного характера
Вы себе противоречите — принудительное лечение назначалось судом, а это уже уголовное преследование.
Ссылку мож найду, а может и нет.
думаю вы видите разницу между административным решанием и уголовным — есть адиминстративыные кодексы, есть уголовные — это две большие разницы
Я конечно не юрист, но большинство административных наказаний — ограничиваются штрафом. Принудительное содержание под стражей, ограничение передвижения или какое-либо изменение состояние человека — принудительная стерилизация, например — принимается по результам уголовных дел, когда предполагается, что человек не сможет себя держать в рамках, определённым законом.
Повторюсь — не юрист.
Зачастую когда приходишь на проект, то уже часть из этих особенностей там используется повсеместно и тратить ресурсы на глобальный рефакторинг никто не желает, поэтому приходится свыкаться.
Вот еще одна статья — разный стиль написания кода в одном проекте.
А стиль, типа
вообще должен влечь за собой максимальную меру
А стиль, типа
function foo() {
function bar() {
echo "hello" ;
}
}
вообще должен влечь за собой максимальную меру
Я подобный стиль видела только на картинках с приколами, а кто-то в реальных проектах с этим встречался?
Не совсем такие, но подобные запутывающие конструкции, увы, встречать приходилось, к примеру:
if (a == b)
doFirstBigFunctionWithMultiplyParameters(a, b, c, d, e, f, g); else
doSecondBigFunctionWithMultiplyParameters(a, b, c, d, e, f, g);
if ((a == b) && isParametersChecked(a, b, c, d)) doFirstFunctionWithParameters(a, b, c, d, e); else
doSecondFunctionWithParameters(a, b, c, d, e, f, g);
4 пункт — вообще лишний, в данной формулировке. Сто раз обсасывалось.
3 пункт — из крайности в крайность. Стремление сократить копипасту и переиспользовать какой-то однострочник отовсюду — вот лютое зло, ибо плодит зависимости на ровном месте.
1 и 2 пункт — тут всё верно, хотя и банально.
3 пункт — из крайности в крайность. Стремление сократить копипасту и переиспользовать какой-то однострочник отовсюду — вот лютое зло, ибо плодит зависимости на ровном месте.
1 и 2 пункт — тут всё верно, хотя и банально.
По 4-му, вам приятно читать функцию по 3 страницы? Что понятнее прочитать:
или
по 3-му, я же сказал — не однострочник, а последовательность действий больше 2х операторов (хотя и два тоже иногда надо убрать). Не обязательно добавлять зависимость на другой модуль, если функция используется только тут можно выделить этот код в приватную функцию. Если данный функционал используется в других модулях, то да — добавлять зависимость на другой объект, но это улучшит тестируемость вашего кода.
function (){
//1-ая страница кода
//2-ая страница кода
//3-я страница кода
}
или
function (){
делаемРаз();
делаемДва();
делаемТри();
}
по 3-му, я же сказал — не однострочник, а последовательность действий больше 2х операторов (хотя и два тоже иногда надо убрать). Не обязательно добавлять зависимость на другой модуль, если функция используется только тут можно выделить этот код в приватную функцию. Если данный функционал используется в других модулях, то да — добавлять зависимость на другой объект, но это улучшит тестируемость вашего кода.
У вас в статье почему то фигурирует:
А тут у нас уже появилось несколько «если». То есть не любой код. И вот про эти «если» многие в порывах чувств забывают.
Для методов «делаем раз, делаем два» — уже то же написали несколько «если» другие комментаторы. Я могу добавить ещё. Дробление сложного метода на много коротких повышает понимание, ЧТО делает метод. Но к сожалению, часто уменьшается понимание КАК он это делает. В особенности, если итоговая вложенность методов сильно больше двух.
Моя претензия к данной статье именно про то, что вы хоть и написали правильные вещи, но отказываетесь рассматривать или забываете про случаи, когда бездумное следование данным правилам только навредит.
любой повторяющийся код содержащий более двух операторов должен быть убран в функцию
А тут у нас уже появилось несколько «если». То есть не любой код. И вот про эти «если» многие в порывах чувств забывают.
Для методов «делаем раз, делаем два» — уже то же написали несколько «если» другие комментаторы. Я могу добавить ещё. Дробление сложного метода на много коротких повышает понимание, ЧТО делает метод. Но к сожалению, часто уменьшается понимание КАК он это делает. В особенности, если итоговая вложенность методов сильно больше двух.
Моя претензия к данной статье именно про то, что вы хоть и написали правильные вещи, но отказываетесь рассматривать или забываете про случаи, когда бездумное следование данным правилам только навредит.
Для методов «делаем раз, делаем два» — уже то же написали несколько «если» другие комментаторы. Я могу добавить ещё. Дробление сложного метода на много коротких повышает понимание, ЧТО делает метод. Но к сожалению, часто уменьшается понимание КАК он это делает. В особенности, если итоговая вложенность методов сильно больше двух.
Вы каждый раз когда добавляете, например, стороннюю зависимость в проект лезете в ее код чтобы понять как она работает? Ну как я уже отвечал ниже — тестирование.
Но когда она сломается неизбежно придется раскручивать цепочку вызовов и при таком расладе функции в одну-две строки немного мешаются, поскольку когда добрался до десятого уровня вложенности, забыл что было на первом.
Не надо забывать про другие эффекты. Вызов функции вещь не бесплатная, поэтому длинные циклы, например, весьма чувтсвительны к большому количеству внутренних вызовов. Для этого есть встраивание функций, но оно не на всех языках доступно.
У меня есть тулза, где используется простыня из switch, преобразовать ее в нечто красивое не получилось после обдумывания с товарищем. Есть переменная, к которой подсоединяются байты и которую нужно проверить, равняется ли она одной из куче дефайнов (кодов 1-4 байта) и в зависимости от этого что-то сделать. Сделать что-то адекватное для уменьшения простыни не представилось возможным.
Старик С99 приговаривается к уголовщине из-за отсутствия синтаксического сахара в свитчах. Примеров еще несколько, думаю.
https://github.com/Izaron/matroska-subtitles/blob/master/matroska.c#L495 (тот пример)
Да с С конечно тяжело, но тоже есть варианты.
У вас там в switch часто повторяется последовательность действий
и еще несколько
их можно в функции,
а коды в массив
double someCodes1[] = {MATROSKA_SEGMENT_TRACK_FLAG_LACING, MATROSKA_SEGMENT_TRACK_FLAG_FORCED, ...};
+ функция isCodeInArray
получится что-то типа
if (isCodeInArray(code, someCodes1))
yourFunc
if (isCodeInArray(code, someCodes2))
yourFunc2
я думаю размер функций уменьшится в несколько раз
P.S. в С не силен
У вас там в switch часто повторяется последовательность действий
read_vint_block_skip(file);
MATROSKA_SWITCH_BREAK(code, code_len);
и еще несколько
их можно в функции,
а коды в массив
double someCodes1[] = {MATROSKA_SEGMENT_TRACK_FLAG_LACING, MATROSKA_SEGMENT_TRACK_FLAG_FORCED, ...};
+ функция isCodeInArray
получится что-то типа
if (isCodeInArray(code, someCodes1))
yourFunc
if (isCodeInArray(code, someCodes2))
yourFunc2
я думаю размер функций уменьшится в несколько раз
P.S. в С не силен
Области видимости, ёктель!
Что приятнее читать:
или
Что приятнее читать:
function (параметр){
//1-ая страница кода
//2-ая страница кода
//3-я страница кода
}
или
function (параметр_один){
...полстраницы кода готовит параметры...
делаемРаз(параметр_один, параметр_два, параметр_три, параметр_четыре, параметр_пять);
...десять строк кода...
}
function делаемРаз(параметр_один, параметр_два, параметр_три, параметр_четыре, параметр_пять) {
...полстраницы кода готовит параметры...
делаемДва(параметр_один, параметр_два, параметр_три, параметр_четыре, параметр_пять, параметр_шесть, параметр_семь, параметр_восемь, параметр_девять);
...десять строк кода...
}
делаемДва(параметр_один, параметр_два, параметр_три, параметр_четыре, параметр_пять, параметр_шесть, параметр_семь, параметр_восемь, параметр_девять, параметр десять) {
...десять строк кода...
}
Скатать по десятку вспомогательных одноразовых функций — сомнительное удовольствие, даже без учета областей видимости.
Передавать объект или, если параметры однотипны, использовать какой-нибудь список.
Это палка о двух концах: общая область видимости — это ещё и риск лишнего взаимодействия.
Но, честно говоря, я считаю, что мыслить нужно гибко: размер функции определяется её спецификой, а не абстрактными правилами.
Но, честно говоря, я считаю, что мыслить нужно гибко: размер функции определяется её спецификой, а не абстрактными правилами.
function (параметр){
//1-ая страница кода
//2-ая страница кода
//3-я страница кода
}В таком виде это тоже плохо.
наверное имелось в виду
function (параметр){
//делаемРаз
//делаемДва
//делаемТри
}Но лучше
//Эта функция выполняется в три этапа
//Раз,Два и Три
function (параметр){
//делаемРаз
страница кода
//делаемДва
ещё страница кода
//делаемТри
ещё много кода
}
И ещё очень важный нюанс, комментарий должен выражать намерение программиста (то что код должен делать). А комментарии, объясняющие что код делает, полезны только в учебниках по программированию.
if (true) конечно неприятно, но гораздо неприятней if (false) — это эволюционировавший криминал из первого пункта про комментарии. еще доставляет exit() или return посреди скрипта/инклюда (речь о пхп) в начале, или лучше где-то посредине файла
Самые страшные из этих грехов — огромные методы и множественность.
Множественность — это самый страшный грех, потому что он делает программу непредсказуемой, а изменения болезненными. Представьте себе простую форму с кнопками, но доступность кнопок меняется в 20 разных методах и в дополнение ко всему ещё и извне модуля. Если программист делая изменения исправит что-то и не заметит хотя бы 1, то мы получаем трудноуловимый баг и кучу головной боли.
К тому же исправить в одном месте и в 20 — это разные трудозатраты (притом даже не в 20 раз, а больше)
Добавляем к этому огромные методы и получаем идеальный вариант проекта, из-за которого хочется уволиться.
Множественность — это самый страшный грех, потому что он делает программу непредсказуемой, а изменения болезненными. Представьте себе простую форму с кнопками, но доступность кнопок меняется в 20 разных методах и в дополнение ко всему ещё и извне модуля. Если программист делая изменения исправит что-то и не заметит хотя бы 1, то мы получаем трудноуловимый баг и кучу головной боли.
К тому же исправить в одном месте и в 20 — это разные трудозатраты (притом даже не в 20 раз, а больше)
Добавляем к этому огромные методы и получаем идеальный вариант проекта, из-за которого хочется уволиться.
Не надо представлять, это страшно! У меня такого (именно такого) тридцать две тысячи строк и полсотни форм. На Дельфи 7. Жесть? Ешё не вся жесть, слушайте дальше.
… С десятками повторяющихся query.sql.add(format(...)[...]); query.sql.add…
… Комментарии? Не знаем таких
… Контроль версий? Некогда… Что, можно совместно работать над одним кодом, над одним файлом? Не верим, не получится. Не сольётся, нет…
… Тесты… Ну да, не забудь прощёлкать форму перед релизом. Табордер проверю!
Впрочем всё это компенсируется следующим: вот тебе одно из приложений в единоличное владение, и делай всё, что считаешь нужным. Так что нет, увольняться не хочу, новое пишу по-новому, старое читать научился.
Но заявление про функции на один экран без уточнения про руководство здравым смыслом и без учёта инструмента и задачи считаю религиозным экстремизмом. Большинство — да. Но сходить с ума за идею я не буду. Не надо путать цель (читаемость) и инструмент (размер). Инструмент надо выбирать с умом и по месту, не так ли?
Но заявление про функции на один экран без уточнения про руководство здравым смыслом и без учёта инструмента и задачи считаю религиозным экстремизмом.
Пока так никто и не показал не математическую функцию больше экрана на языке высокого уровня, которую нельзя сократить.
«Математическую» — ложное предположение, заведомо ложное. Прикладное программирование отнюдь не ограничивается математикой. И между «можно» и «нужно», насколько мне известно, пропасть. Бывает и можно, и лучше станет, но не настолько же лучше, насколько дороже. И цель (читаемость) !== средству (выносу мозга… Нет, части мозга… Или мозга по частям в отдельные черепа, простите за такое сравнение). А утверждение, что некое правило поведения априори не должно иметь исключений, пусть так делают все, и давайте не будем про здравый смысл — это и есть религиозный экстремизм. Я не утверждаю, что есть методы, которые нельзя безвредно с точки зрения понятности разбить (хотя про sql запросы таки да, утверждаю), и примеров не приведу. Но я утверждаю, что это не всегда оправдано в несферических обстоятельствах, начиная от дефицита ресурсов (время, квалификация и т.п.) и заканчивая тем самым «цена/качество».
Закомментированный код
Как часто в проектах любого уровня это встречается, человек как бы говорит вам: я немного не уверен, может быть это еще понадобится, а может и нет я — хз, просто оставлю это пока тут. Типичный кодомусорщик. Есть даже целые закомменченные модули. Это опять же очень сильно влияет на читаемость кода, даже несмотря на то, что он идет другим цветом. Решение есть — просто удалить, и все.
В принципе неверное решение!
Комментирование кода используется по причине отсутствия хорошо работающего удобного контроля кода. И удалять такой код — это лечить головную боль топором. В случае необходимости откатиться на одну из предыдущих версий кода, или просто необходимости его посмотреть, будет много заслуженного мата в адрес того кто удалил этот код.
=> правильное решение, это организовать удобный хорошо работающий контроль версий кода. И тогда отпадёт необходимость хранить неактуальный код внутри актуального.
Заминусуйте меня до смерти, но я, бывает, предпочитаю не длинные, а очень длинные методы.
Банальный пример. Допустим, вытаскиваем мы что-то из какой-то внешней системы. Обычная ETL-задачка. Для начала нужно проверить параметры, потом установить соединение, сформировать запрос, получить ответ, распарсить ответ, сопоставить сущности, выполнить кучу разных трансформов, записать данные, запротоколировать. Если вытянуть всё в одну колбасу, получается, например, 5 тыс. кода. Выносим повторяющиеся куски, получаем 2 тыс. строк основного метода плюс 1 тысяча строк вспомогательных. Есть ли смысл в погоне за снижением цикломатической сложности нарезать оставшиеся 2 тыс. на мелкие кусочки, если заведомо известно, что они в качестве самостоятельных единиц точно больше нигде и ни для чего не понадобятся? От слова «никогда»?
Некоторые задачи имеют чётко выраженный характер длинного последовательного нарратива. Притом заведомо уникального. Какой интерес в том, чтобы вместо романа писать сборник не имеющих самостоятельного смысла ультракоротеньких рассказов, а сам роман делать в виде оглавления?
Когда мне надо написать «роман в коде», я просто в его начало вписываю описание сюжета, а потом размечаю повествование. Как-то так:
Встречался мне код, написанный разрабами, помешанными на снижении цикломатической сложности. Рефакторить его — проще выбросить.
Банальный пример. Допустим, вытаскиваем мы что-то из какой-то внешней системы. Обычная ETL-задачка. Для начала нужно проверить параметры, потом установить соединение, сформировать запрос, получить ответ, распарсить ответ, сопоставить сущности, выполнить кучу разных трансформов, записать данные, запротоколировать. Если вытянуть всё в одну колбасу, получается, например, 5 тыс. кода. Выносим повторяющиеся куски, получаем 2 тыс. строк основного метода плюс 1 тысяча строк вспомогательных. Есть ли смысл в погоне за снижением цикломатической сложности нарезать оставшиеся 2 тыс. на мелкие кусочки, если заведомо известно, что они в качестве самостоятельных единиц точно больше нигде и ни для чего не понадобятся? От слова «никогда»?
Некоторые задачи имеют чётко выраженный характер длинного последовательного нарратива. Притом заведомо уникального. Какой интерес в том, чтобы вместо романа писать сборник не имеющих самостоятельного смысла ультракоротеньких рассказов, а сам роман делать в виде оглавления?
Когда мне надо написать «роман в коде», я просто в его начало вписываю описание сюжета, а потом размечаю повествование. Как-то так:
// 1. Делаем разИ нормально. Никто не жаловался.
// 2. Делаем два
// 3.…
// (1) Делаем раз
…
// (2) Делаем два
...
Встречался мне код, написанный разрабами, помешанными на снижении цикломатической сложности. Рефакторить его — проще выбросить.
Есть ли смысл в погоне за снижением цикломатической сложности нарезать оставшиеся 2 тыс. на мелкие кусочки, если заведомо известно, что они в качестве самостоятельных единиц точно больше нигде и ни для чего не понадобятся? От слова «никогда»?
Тестирование, подумайте, насколько легче тестировать эти самые разрозненные кусочки, в отличие от монолита.
Монолит — один предмет тестирования, а сотня его ошмётков — соответственно сотня предметов. Не забываем о рефакторинге. В какой-то момент может оказаться, что ключевую логику (допустим, это 50% объёма кода) можно ускорить на порядок, если сделать не вдоль, а поперёк. Что будем с тестами делать?
Поскольку монолит проще ваять и проще понимать (если в нём подробно и грамотно всё откомментировано), он получается и оптимальнее (мозг разработчика не отвлекается на выдумывание ненужной разбивки), проще в отладке и проще в реагировании на изменения. Следствие этой простоты — надёжность.
Да, понять метод в 2000 строк не просто. Но понять 100 методов по 20 строк — вообще катастрофа. Может быть, отчасти из-за этого и получается, что "разработчик 95% времени читает свой или чужой код и только 5% пишет"?
Поскольку монолит проще ваять и проще понимать (если в нём подробно и грамотно всё откомментировано), он получается и оптимальнее (мозг разработчика не отвлекается на выдумывание ненужной разбивки), проще в отладке и проще в реагировании на изменения. Следствие этой простоты — надёжность.
Да, понять метод в 2000 строк не просто. Но понять 100 методов по 20 строк — вообще катастрофа. Может быть, отчасти из-за этого и получается, что "разработчик 95% времени читает свой или чужой код и только 5% пишет"?
UFO just landed and posted this here
Во многих случаях достаточно покрыть публичные методы.А публичный-то метод у нас один. Собственно, сам монолит.
Конечно, ко всему нужно подходить с умом. Если есть смысл обвесить кусок отдельным юнит-тестом, то конечно его нужно вынести. Я же ни в коем случае не выступаю против обоснованной фрагментации. Я против фрагментации, мотивированной абстрактным формальным критерием снижения цикломатической сложности.
особенно если им давать семантически значимые названияВот с этим обычно вообще беда. Варианты в стиле «Посушить_клиенту_голову_после_стрижки_ежиком_если_он_пришел_по_рекомендации_васи» — форменный кошмар.
UFO just landed and posted this here
Метод внутри метода в случае его заведомо однократного использования не сокращает длину кода внешнего метода. Даже немножко увеличивает. То есть количество страниц останется примерно тем же.
Но как способ сделать так, чтобы враги не догадались, как оно работает, это определённо можно рекомендовать :))
Но как способ сделать так, чтобы враги не догадались, как оно работает, это определённо можно рекомендовать :))
UFO just landed and posted this here
Аналогия про оглавление не совсем правильная. В оглавление сводится вся структура, а здесь мы имеем в основном оглавлении только список глав, а список разделов глав — в самих главах, подразделов — в разделах и дальше вглубь. Получается что-то вроде такого:
Как понятность? Нормально? Это у нас ещё без циклов и ветвлений. И по модулям не разбросано. И ни одного прекрасного паттерна ООП не применено.
Так и живём :(
В дополнение к этому нужно понимать, что кроме control flow есть ещё data flow. Курочка инициализируется в 1-й строке функции «ПолучениеЯичка», там же юзается, а потом ещё юзается в функциях «Утешение» и «Обещание». С объектом «Яичко» тоже не всё просто.
def Книга:
Жили были дед да баба
ПолучениеЯичка()
ЯичкоРазбито()
Горе()
Утешение()
def Обещание:
Снесу я вам яичко
Не золотое, а простое
def УдачнаяПопыткаПолученияРезультата:
Мышка бежала
Хвостиком махнула
Яичко упало и разбилось
def ЯичкоРазбито:
ПерваяПопыткаПолученияРезультата()
УдачнаяПопыткаПолученияРезультата()
def Утешение:
Не плачь дед, не плачь баба
Обещание()
def ПолучениеЯичка:
Была у них курочка Ряба
Снесла она яичко не простое, а золотое
def Горе:
Плачет дед
Плачет баба
def ПерваяПопыткаПолученияРезультата:
Дед бил, бил, не разбил
Баба била, била, не разбила
Как понятность? Нормально? Это у нас ещё без циклов и ветвлений. И по модулям не разбросано. И ни одного прекрасного паттерна ООП не применено.
Так и живём :(
В дополнение к этому нужно понимать, что кроме control flow есть ещё data flow. Курочка инициализируется в 1-й строке функции «ПолучениеЯичка», там же юзается, а потом ещё юзается в функциях «Утешение» и «Обещание». С объектом «Яичко» тоже не всё просто.
UFO just landed and posted this here
Пример не дам. Даже не просите. Примеры, вспоминаемые навскидку, частично не мои хотя бы в части постановки задачи. Это напрочь исключает публикацию.
Семантическая связанность — норма жизни. Тот же самый ETL уже на этапе извлечения настроек интеграции даёт некоторый объём данных, которые могут использоваться по сути в любой точке алгоритма. По мере продвижения к концу объём контекста только растёт.
Естественно, разбираться в 2000 строчках кода сложнее, чем в 20. Кто бы спорил? Но если 20 умножить на 100, то 2000 не получится. Добавится необходимость помнить, как эта беда складывается в дерево вызовов. То есть помнить, что «Мышка бежала» получает управление через функцию «ЯичкоРазбито», которая стартует после «Снесла она яичко не простое, а золотое» в функции «ПолучениеЯичка» и перед «Не золотое, а простое» в функции «Обещание».
Семантическая связанность — норма жизни. Тот же самый ETL уже на этапе извлечения настроек интеграции даёт некоторый объём данных, которые могут использоваться по сути в любой точке алгоритма. По мере продвижения к концу объём контекста только растёт.
Естественно, разбираться в 2000 строчках кода сложнее, чем в 20. Кто бы спорил? Но если 20 умножить на 100, то 2000 не получится. Добавится необходимость помнить, как эта беда складывается в дерево вызовов. То есть помнить, что «Мышка бежала» получает управление через функцию «ЯичкоРазбито», которая стартует после «Снесла она яичко не простое, а золотое» в функции «ПолучениеЯичка» и перед «Не золотое, а простое» в функции «Обещание».
def Книга:
Жили были дед да баба
ПолучениеЯичка()
ЯичкоРазбито()
Горе()
Утешение()
def ПолучениеЯичка:
Была у них курочка Ряба
Снесла она яичко не простое, а золотое
def ЯичкоРазбито:
ПерваяПопыткаПолученияРезультата()
УдачнаяПопыткаПолученияРезультата()
def ПерваяПопыткаПолученияРезультата:
Дед бил, бил, не разбил
Баба била, била, не разбила
def УдачнаяПопыткаПолученияРезультата:
Мышка бежала
Хвостиком махнула
Яичко упало и разбилось
def Горе:
Плачет дед
Плачет баба
def Утешение:
Не плачь дед, не плачь баба
Обещание()
def Обещание:
Снесу я вам яичко
Не золотое, а простое
> Как понятность? Нормально?
Да, вполне. Если ещё и методы не называть ПерваяПопыткаПолученияРезультата — ещё лучше будет.
> Так и живём :(
Преднамеренно запутывая код? )
А параметры (Дед, Баба, Мышка, Курочка, Яичко и проч.) в функции кто передавать будет — Пушкин? Или на глобальных переменных всё делать будете?
UFO just landed and posted this here
Лично я, работая плотно с базами данных в энтерпрайзе, таких портянок не видел. Мат начинался строк с 300.
А вот чисто на SQL портянки видел, и мне вообще не понравилось. Но на SQL это скорее в силу скудности инструментария.
А мой оппонент пример понятной портянки привести отказывается, вместо этого троллит, притащив курочку рябу, с мешаниной ужасно названных методов. может Вы покажете о чем речь?
А вот чисто на SQL портянки видел, и мне вообще не понравилось. Но на SQL это скорее в силу скудности инструментария.
А мой оппонент пример понятной портянки привести отказывается, вместо этого троллит, притащив курочку рябу, с мешаниной ужасно названных методов. может Вы покажете о чем речь?
Преднамеренно код никто не запутывает. Он сам прекрасно запутывается. А делать то, что вы сделали, то есть выстраивать в порядке исполнения — помогает мало.
Мы все на уровне автоматизма знаем, что после того, как отрабатывается последняя строчка функции, управление уходит в вызывающую функцию, а не перескакивает на первую строчку следующей по порядку в тексте. То есть после выполнения «Плачет баба» у нас следующей строчкой будет… что? Смотрим, ищем… ага, после вызова «Горе» у нас вызов функции «Утешение», а там у нас первым делом «Не плачь дед, не плачь баба». То, что эта строчка в тексте модуля сразу после «Плачет баба» — это обязано нам ничего не говорить о порядке выполнения.
Мы все на уровне автоматизма знаем, что после того, как отрабатывается последняя строчка функции, управление уходит в вызывающую функцию, а не перескакивает на первую строчку следующей по порядку в тексте. То есть после выполнения «Плачет баба» у нас следующей строчкой будет… что? Смотрим, ищем… ага, после вызова «Горе» у нас вызов функции «Утешение», а там у нас первым делом «Не плачь дед, не плачь баба». То, что эта строчка в тексте модуля сразу после «Плачет баба» — это обязано нам ничего не говорить о порядке выполнения.
> помогает мало
Зачем же Вы тогда перемешали, если разницы нет? Сначала хотели запутать, а теперь говорите, что помогает мало?
Что ещё мало помогает? Нормально методы называть? Выдерживать стандарты кодирования? Не делать однобуквенных переменных?
Если вы не поддерживаете элементарного порядка в коде — конечно у вас всё не понятно.
Вы решили, что «обязано нам ничего не говорить» и макароны в коде, а у меня конвенция, и её соблюдение мне «говорит» и делает код предсказуемым.
Зачем же Вы тогда перемешали, если разницы нет? Сначала хотели запутать, а теперь говорите, что помогает мало?
Что ещё мало помогает? Нормально методы называть? Выдерживать стандарты кодирования? Не делать однобуквенных переменных?
Если вы не поддерживаете элементарного порядка в коде — конечно у вас всё не понятно.
Вы решили, что «обязано нам ничего не говорить» и макароны в коде, а у меня конвенция, и её соблюдение мне «говорит» и делает код предсказуемым.
Господа, что-то мы уехали в какие-то литературные примеры. Дайте вариант реального кода, где реально 4 страницы кода нельзя сделать более читаемыми (а возможно даже меньше по размеру), разбив на функции. Вот мой пример, который я приводил ниже. Я утверждаю, что сейчас там почти ничего не понятно, сделав функцию меньше, например выделив результаты промисов ( код который идет внутри then ) в отдельные функции, читаемость повысится в разы. Приведите реальный пример реальной простыни, которую нельзя разбить.
С другой стороны найти и отладить ошибку в 20 строках кода легче, чем в 2000 строках кода. Да и не нужно понимать 100 методов, т.к. что они делают должно понятно из названия / комментариев, а как они это делают — это уже проблема самих методов и туда нужно закапываться только если вы поняли что проблема в нём. А вот для 2000 строк кода — нужно держать в голове всю ветку условий и набор параметров.
Чаще всего, ошибка не в 20 строчках и не в 2000, а в одной. Иногда её проще отладить в длинной портянке, иногда целесообразно вытащить в отдельно отлаживаемый кусок. Если второе, то вынести. Почему бы и нет? Главное — подходить с умом к процессу и не делать фетиш ни из мелкокусочечной нарезки, ни из монолитостроения.
А вот для 2000 строк кода — нужно держать в голове всю ветку условий и набор параметров.А их всё равно держать придётся. Кусок в 20 строк всё равно обретает смысл только в контексте всей задачи. Если задача сама по себе сложна, то нет никакого способа уменьшить сложность её реализации. Да, каждый отдельный маленький метод прост, но выполнение задачи даёт не каждый отдельный метод, а их полный комплекс. Нарезка монолита на 100 кусочков очень не способствует пониманию единого целого во всей полноте.
> Нарезка монолита на 100 кусочков очень не способствует пониманию единого целого во всей полноте.
Для вырезания куска принципиально наличие абстракции, понимание внутренностей которой на понимание единого целого не влияет.
Для «понимания во всей полноте» надо не только свои функции инлайнить, но и, как минимум, функции фреймворка и ОС.
Для вырезания куска принципиально наличие абстракции, понимание внутренностей которой на понимание единого целого не влияет.
Для «понимания во всей полноте» надо не только свои функции инлайнить, но и, как минимум, функции фреймворка и ОС.
Для вырезания куска принципиально наличие абстракции, понимание внутренностей которой на понимание единого целого не влияет.Если такая абстракция есть, то нам свезло. Смело вырезаем кусок и радуемся жизни. Но если такую абстракцию приходится высасывать из пальца, то лучше не надо.
Вы так говорите, как будто полный список абстракции Господь создал на восьмой день.
Выделять абстракции — это навык.
Выделять абстракции — это навык.
А выделять абстракции там, где их нет, ради веры в то, что читаемость === один экран — это уже религиозный экстремизм :).
Разные задачи, разные инструменты, разные критерии понятности, да даже экраны у всех разные. А метод всё равно что-то "должен". А я считаю, что раз расписки нет, то и не должен ничего :))
1) А я разве где-то утверждал, что должен?
2) Опять же, разве я где-то писал, что «читаемость === один экран»? При прочих равных, один экран прочитать проще, чем два (надеюсь это не надо доказывать?). Дальше вопрос только в том, сможете ли Вы написать в один экран, хватит ли Вам квалификации под конкретную задачу с заданными инструментами. Квалификацию и инструменты мы, теоретически, можем улучшать. Может быть даже можно переформулировать задачу. А можно написать спагетти и сказать «мне так понятнее».
3) Может быть Вы покажете как же выглядит понятная простыня без абстракций? А то двое уже слились. И на моей практике такие пространные рассуждения про критерии понятности выливаются в банальный говнокод.
Бывает, что написать лучше не получается. Бывает, что простыню на 2 экрана стираешь, заменив на вызов пары методов.
Но вот что бы на 2000 строк не нашлось абстракций — не видел.
2) Опять же, разве я где-то писал, что «читаемость === один экран»? При прочих равных, один экран прочитать проще, чем два (надеюсь это не надо доказывать?). Дальше вопрос только в том, сможете ли Вы написать в один экран, хватит ли Вам квалификации под конкретную задачу с заданными инструментами. Квалификацию и инструменты мы, теоретически, можем улучшать. Может быть даже можно переформулировать задачу. А можно написать спагетти и сказать «мне так понятнее».
3) Может быть Вы покажете как же выглядит понятная простыня без абстракций? А то двое уже слились. И на моей практике такие пространные рассуждения про критерии понятности выливаются в банальный говнокод.
Бывает, что написать лучше не получается. Бывает, что простыню на 2 экрана стираешь, заменив на вызов пары методов.
Но вот что бы на 2000 строк не нашлось абстракций — не видел.
Не всегда.
Есть, например, метод, который опрашивает возможности железки по выполнению какой-нибудь задачи (аппаратный кодек или что-то вроде того). В этом единственное предназначение железки, смысл её существования и сценарий использования.
Каков смысл растаскивать эту простыню на несколько частей, если использовать можно только одним целым? Ездить туда-сюда по колл стеку?
Есть, например, метод, который опрашивает возможности железки по выполнению какой-нибудь задачи (аппаратный кодек или что-то вроде того). В этом единственное предназначение железки, смысл её существования и сценарий использования.
Каков смысл растаскивать эту простыню на несколько частей, если использовать можно только одним целым? Ездить туда-сюда по колл стеку?
Если IDE поддерживает Inline Method и Extract Method рефакторинг не пугает.
Понятно. Сначала собрать монолит из кусочков, а потом опять его разобрать. После третьей итерации шаг «разобрать» делать уже не захочется.
Зачем монолит переколбашивать 3 раза? Три раза «может оказаться, что ключевую логику можно ускорить на порядок, если сделать не вдоль, а поперёк»?
Если вы переписываете одну из глав — ту пару функций и пересобираете. Если связность не позволяет и надо переписывать всё кучей, то… Надо уменьшать связность.
А уж комментарии в таком монолите безвозвратно устаревают после первого же переписывания
Если вы переписываете одну из глав — ту пару функций и пересобираете. Если связность не позволяет и надо переписывать всё кучей, то… Надо уменьшать связность.
А уж комментарии в таком монолите безвозвратно устаревают после первого же переписывания
Это смотря как пользоваться. На прошлой работе был коллега, который писал длинный метод, а затем бездумно выделял куски кода и делал Extract Method. Получал методы с кучей параметров, из которых половина — out и ref (речь о C#). Поддерживать Это было невозможно.
Для наглядности приведу один из реальных кусков такой работы.
private void MakeWorkflow(WorkAndRestInputArgumentsBase arguments,
IList<DriverActivityFullRecord> records,
IList<DriverSpecificConditionRecord> driverSpecificConditionRecords)
{
List<DriverActivityFullRecord> filteredDublicateDAFR;
List<DriverActivityInterval> driverActivityIntervals;
IList<TimeRange> ferryOrTrainCrossingIntervals;
MakeWorkflow_Part1(arguments as WorkAndRestInputArgumentsEuropean, records, driverSpecificConditionRecords,
out filteredDublicateDAFR, out driverActivityIntervals, out ferryOrTrainCrossingIntervals);
if (driverActivityIntervals.Count <= 0 || filteredDublicateDAFR.Count <= 0)
return;
bool isINSPTransportation;
List<TimeRange> weeks;
List<TimeRangeWithDriverActivityIntervals> weeksWithActivityIntervals;
List<RestTimeInfo> recreations;
List<WorkWeekRests> workWeekRests;
GenericRange<DateTime> dataRange;
MakeWorkflow_Part2(arguments as WorkAndRestInputArgumentsEuropean, driverActivityIntervals,
filteredDublicateDAFR, ferryOrTrainCrossingIntervals,
out isINSPTransportation,
out weeks,
out weeksWithActivityIntervals,
out recreations,
out workWeekRests,
out dataRange);
}
Мне кажется, что после того как вы выделили «главы» в своём «романе» — целесообразно вынести каждую «главу» в отдельный метод. Т.е. если даже код не будет вызываться повторно (и будет выполняться строго линейно), то всё-равно удобнее работать с небольшим куском кода, который имеет чёткое предназначение, чем копаться в портянке на 5000 строк кода и мешанине условий и переменных.
А вообще в большинстве случаев портянка кода говорит о непродуманной архитектуре модуля (возможно нужен какой-нибудь паттерн вроде Visitor-а)
А вообще в большинстве случаев портянка кода говорит о непродуманной архитектуре модуля (возможно нужен какой-нибудь паттерн вроде Visitor-а)
Некоторые задачи имеют чётко выраженный характер длинного последовательного нарратива.… Есть ли смысл в погоне за снижением цикломатической сложности нарезать оставшиеся 2 тыс. на мелкие кусочки
Цикломатическая сложность последовательного (т.е. без циклов и ветвлений) кода минимальна и равна единице, уменьшить её можно только удалив код полностью :)
Цикломатическая сложность последовательного (т.е. без циклов и ветвлений) кода минимальна и равна единице, уменьшить её можно только удалив код полностью :)
Цикломатическая сложность последовательно идущих кусочков складывается методом суммирования из сложностей каждого из этих кусочков.
Что вы подразумеваете под «кусочками»? 100 идущих подряд арифметических операций — это 100 кусочков, или один? А 100 последовательных вызовов разных функций? Если каждая функция имеет сложность 1, то 100 последовательных вызовов даст сложность 100, по-вашему?
Монолит:
Порезали:
Максимальная цикломатическая сложность была 5, а стала 3. Типа профит.
function big_func() { // CC = 5
for ... {
for ... {
...
}
}
if ... {
for ... {
...
}
} else {
...
}
}
Порезали:
function big_func() { // CC = 1
small_func1();
small_func2();
}
function small_func1() { // CC = 3
for ... {
for ... {
...
}
}
}
function small_func2() { // CC = 3
if ... {
for ... {
...
}
} else {
...
}
}
Максимальная цикломатическая сложность была 5, а стала 3. Типа профит.
Во-первых, суммирования тут нет — если бы было суммирование, то получилось бы 3+3=6.
Во-вторых, вы сами написали, что код — последовательный нарратив, без циклов и ветвлений. И выше даже привели пример про дедушку и яичко, у которого цикломатическая сложность как была 1, так и осталась. А здесь у вас уже и циклы есть, и условия.
Во-вторых, вы сами написали, что код — последовательный нарратив, без циклов и ветвлений. И выше даже привели пример про дедушку и яичко, у которого цикломатическая сложность как была 1, так и осталась. А здесь у вас уже и циклы есть, и условия.
И ваш подход будет нормально выглядеть… Пока какому-нибудь разработчику не понадобится подправить функционал в этом методе. И вот он будет сидеть, скроллить код и искать, где же то, что ему нужно. И, классически, этим разработчиком будете вы через несколько месяцев.
Если разбивка такой системы на методы по 20 строк кода делает ваш код менее читабельным, то стоит задуматься, а умеете ли вы писать нормально методы?
Если разбивка такой системы на методы по 20 строк кода делает ваш код менее читабельным, то стоит задуматься, а умеете ли вы писать нормально методы?
Пока какому-нибудь разработчику не понадобится подправить функционал в этом методе.По-разному бывает. По опыту скажу, что докручивать «дополнительные вводные» в монолите проще и быстрее. И более предсказуемый результат. И меньше риска поймать побочку. Хотя, конечно, по-разному бывает.
Простой пример. Допустим, нужно на каком-то шаге сделать доп. проверку. А для этого нужно слазить в базу. Если дизайн мелкогранулированный, то находим нужную процедуру и ничтоже сумняшеся вписываем запрос к базе. Включаем — адские тормоза. А почему? А потому, что это только с виду у нас запрос на верхнем уровне кода, а реально он в трёх вложенных циклах. Если же мы работаем с монолитом, то сразу видим внешнее окружение и в мозгу автоматически срабатывает триггер «запрос в цикле снег голова попадёт».
В таком случае если тормоза адские, то можно считать, что повезло, потому что сразу заметили и сразу исправили. Но часто бывает, что из-за такой же или подобной пакости снижение скорострельности получается маленькое и на глаз не заметное. Процентов на 10. Потом в схожей ситуации добавится ещё 10, потом ещё и ещё. В результате имеем, что программка, которая раньше летала на Pentium-3 теперь тормозит на Core i7. Хотя функциональности добавлено с гулькин нос. Начинаем смотреть что плохо, и понимаем, что перепахивать нужно всё. А на это уже ни ресурсов, ни желания :(
UFO just landed and posted this here
По-правильному (предположим, изначально так и было сделано) все исходные данные для проверок извлекаются из базы заранее, вне циклов, и передаются вызываемым методам по цепочке вызовов. В конечном счёте выглядит всё вполне невинно: например, в метод прилетают параметры типа ItemID, Quantity, Summ, QExist. Метод сравнивает Quantity c QExist, и если нет превышения, то пускает выполнение по одной ветке, а если есть, то по другой. Предположим, в базу добавили флажок «Запретить отгрузку». Соответственно, если он для ItemID выставлен, то выполнение пускается по «обломной» ветке. Для добычи значения флажка уже есть метод GetStoppedFlagForItemID(), и вызов этого метода мы, не подумав, вписываем в условие. Здравствуй, тормоз.
Заранее соглашусь с тем, что нужно делать хорошо, и не нужно плохо. Даже понятно как именно. Но реальность такова, что сделать надо срочно, разбираться некогда, и вообще дело поручено товарищу, который видит этот код в первый раз. Спасибо хоть, что нашёл в мешанине методов то место, в которое нужно вписать условие. А разматывание цепочки вызовов — отдельная задача, и далеко не факт, что у товарища хватит любопытности этим себя развлечь.
К сожалению, это всё рядовые ситуации. Наверно, в любой системе, имеющей богатую историю, есть куча кусков, которые на первый взгляд выглядят как конфетки, а на самом деле самое настоящее дерьмо. В монолите хотя бы сразу бывает видно, что на диск или в сеть полезли совсем не там, где следовало.
Заранее соглашусь с тем, что нужно делать хорошо, и не нужно плохо. Даже понятно как именно. Но реальность такова, что сделать надо срочно, разбираться некогда, и вообще дело поручено товарищу, который видит этот код в первый раз. Спасибо хоть, что нашёл в мешанине методов то место, в которое нужно вписать условие. А разматывание цепочки вызовов — отдельная задача, и далеко не факт, что у товарища хватит любопытности этим себя развлечь.
К сожалению, это всё рядовые ситуации. Наверно, в любой системе, имеющей богатую историю, есть куча кусков, которые на первый взгляд выглядят как конфетки, а на самом деле самое настоящее дерьмо. В монолите хотя бы сразу бывает видно, что на диск или в сеть полезли совсем не там, где следовало.
Само существование GetStoppedFlagForItemID, который лезет в базу за одним флагом — это уже ошибка. Которую очень просто недопустить!
Если все данные исходно загружаются из базы в одном месте — то надо просто добавить запрос флага в то же самое место, вот и все. Это даже проще, чем делать еще одну функцию.
Само существование GetStoppedFlagForItemID, который лезет в базу за одним флагом — это уже ошибка. Которую очень просто недопустить!Не поверите, но бывает так, что такие фокусы бывают частью базового фреймворка. Добавляем в таблицу
Item флажок Stopped, и сразу в коде можем писать if (MyItem.Stopped).... Обалденно удобно, если не злоупортеблять.Как сделать правильно, я знаю. И вы знаете. И все знают. И как не нужно делать, тоже все знают, и все готовы поклясться страшной клятвой, что отныне никогда и ни за что. Но когда дело доходит до дела, открываем код, многократно насилованный поколениями сопровожденцев, и рука тянется к пулемёту. Все хорошие мальчики и девочки, но если ошибочность доделываемого явно не бросается в глаза, никто дополнительно напрягаться проводить раскопки не будет. Допилили, прогнали тесты, вроде работает, ну и ладно. В релиз.
Вот именно. Проверяем заранее загруженный флажок if (MyItem.Stopped) — и никаких лишних запросов к базе! Им просто неоткуда взяться.
UFO just landed and posted this here
Затем, что в ситуации "лень разбираться" лучше загрузить по умолчанию все данные сразу, чем делать два запроса.
Для этого надо:
а). Найти место, где данные грузятся.
б). Понять, почему там не грузится Stopped. Может быть, это сделано умышленно? Может быть, где-то в недрах бороды методов этот флаг может быть скорректирован, и тогда мы из кэша вынем неактуальное значение?
в). Если всё ОК, то дописать извлечение флага.
г). Дотянуть этот флаг от места добычи до места использования.
В режиме «fuck cyclomatic complexity delusion» пункты (а) и (б) сильно проще, пункт (в) без разницы, а пункт (г) вообще не нужен.
а). Найти место, где данные грузятся.
б). Понять, почему там не грузится Stopped. Может быть, это сделано умышленно? Может быть, где-то в недрах бороды методов этот флаг может быть скорректирован, и тогда мы из кэша вынем неактуальное значение?
в). Если всё ОК, то дописать извлечение флага.
г). Дотянуть этот флаг от места добычи до места использования.
В режиме «fuck cyclomatic complexity delusion» пункты (а) и (б) сильно проще, пункт (в) без разницы, а пункт (г) вообще не нужен.
UFO just landed and posted this here
Про флажок в базе данных не совсем понялНе суть. В любом случае для того, чтобы выяснить, вызовет ли беду втыкание длительной операции в конкретное место кода, нужно разобраться во внешнем окружении. В портянке оно, как правило, виднее, чем в отдельно болтающемся маленьком методе с невинным именем.
Если ваш монолит предназначен быстро решить какую-то задачу и отправиться в корзину или гаранитрованно не требует дальнейшей поддержкиКак раз лично моя практика сопровождения/развития систем свидетельствует в пользу того, что необоснованное раздробление кода сильно мешает и поддержке, и развитию.
А как вы в портянке видите, что GetStoppedFlagForItemID вызывать не надо?
Я не совсем лох, и в момент написания кода мне известно, что этот вызов тянет за собой SQL-запрос. Простой, быстрый, но всё же тянет. И ещё я вижу хотя бы чисто по отступам, что оно внутри трёх вложенных циклов. Ситуация «запрос в цикле» опознаётся автоматически.
> И ещё я вижу хотя бы чисто по отступам, что оно внутри трёх вложенных циклов.
Кроме синтаксиса есть семантика. Хотя бы по названию метода GetStoppedFlagForItemID видно, что он, в 99 случаях из 100, будет вызван из цикла.
А там уже 2 шага до удаления подобных методов с айтемов под лозунгом aggregate roots. Нет метода — нет проблемы.
Кроме синтаксиса есть семантика. Хотя бы по названию метода GetStoppedFlagForItemID видно, что он, в 99 случаях из 100, будет вызван из цикла.
А там уже 2 шага до удаления подобных методов с айтемов под лозунгом aggregate roots. Нет метода — нет проблемы.
На sql у меня не будет такого метода. Зато будет запрос по всем item (все равно нужный для работы), к которому joinом прицепится колонка lIsStopped. И упаси меня боже от скалярной функции: это будет именно join или просто колонка из таблицы. Потому — "мальчики, не ссорьтесь" (ц) Мальвина.
Такой метод появляется не на пустом месте, а потому, что флаг лежит, например, в другой БД, и join просто не работает.
На всякий случай: ситуация с ItemID и Stopped — целиком выдуманная. Бывает так, что в функцию, вызываемую в цикле, вклячивается длинный запрос, цепляющий через join десяток таблиц. Так что на Stopped, пожалуйста, не фиксируйтесь.
Вы kolu4iy хотели ответить? Я не сомневался, что бывает. Я сомневался, что с «автоматическим опознаванием» такой ситуации есть сложности.
С подобными ситуациями обычно помогает бороться документ, который можно озаглавить, например, «Концепция разработки», с коэффициентами премирования. Обычно людям обидно регулярно получать на 10-15% меньше, чем их коллега, просто потому что они допустили подобный вариант развития событий. Ну и время на рефакторинг, в случае выявления подобных ситуаций, обойдётся дешевле, чем запустить подобною многовложку в продакшн и смотреть как плачет бизнес.
Почему метод проверки сам лезет в базу? Он должен получить все нужные ему для проверки данные на входе
А что, если лезть в базу за данными для доп. проверки надо только в 0.01% случаев, а запрос к базе занимает 10 секунд? Всё равно делать запрос каждый раз и давать на вход?
Думаю проблема при таком подходе не столько в длинне функции, сколько во внутренней связанности. Если в десятом по ходу действии используются переменные объявленные в действиях 1 и 2, становится сложновато дебажить.
Да ладно дебажить. При таком подходе невозможно без скрола увидеть тип переменной!
… а некоторые еще любят перед простыней кода на десяток страниц объявить все переменные в начале функции.
Смысл последовательной портянки зачастую бывает в том, что мы по ходу пьесы последовательно копим мощный контекст, который в полной мере используется на последних шагах. Если распилить монолит, то этот контекст придётся протаскивать через параметры.
В мелко нарезанной обработке данных искать ответ на вопрос типа «где же так криво вычислился вот этот параметр?» бывает на редкость геморно. Оно и так-то бывает не просто, но прыжки через длиннющие цепочки методов оптимизма точно не прибавляют.
В мелко нарезанной обработке данных искать ответ на вопрос типа «где же так криво вычислился вот этот параметр?» бывает на редкость геморно. Оно и так-то бывает не просто, но прыжки через длиннющие цепочки методов оптимизма точно не прибавляют.
Обычно такой контекст закапывают внутрь класса или в структуру, которую явно передают от блока к блоку.
Вовнутрь класса — отстой. Все эти промежуточные данные не должны оседать в состояние объекта.
Сделать структуру «Контекст» и таскать её с собой повсюду — годное решение. Сам сто раз так делал. Позволяет не плодить методы со списком параметров на три экрана. Жаль только, что в вопросе про происхождение кривого значения это ничем не помогает. Вместо «где же так криво вычислился вот этот параметр» имеем «где же так криво вычислилось вот это свойство контекста».
Сделать структуру «Контекст» и таскать её с собой повсюду — годное решение. Сам сто раз так делал. Позволяет не плодить методы со списком параметров на три экрана. Жаль только, что в вопросе про происхождение кривого значения это ничем не помогает. Вместо «где же так криво вычислился вот этот параметр» имеем «где же так криво вычислилось вот это свойство контекста».
«где же так криво вычислился вот этот параметр?»
это сложно если методы мутируют контекст.
можно сделать большой метод вот таким:
var context = new Context();
context.Prop1 = GetProp1Method(context);
context.Prop2 = GetProp2Method(context);
ActOnFullContext(context);
GetProp1Method(IGetProp1Abstraction input);
GetProp2Method(IGetProp2Abstraction input);
public class Context: IGetProp1Abstraction, IGetProp2Abstraction {
public int Prop1 {get;set;}
public int Prop2 {get;set;}
}
public interface IGetProp1Abstraction {
object SomethingElse {get;}
}
public interface IGetProp2Abstraction {
int Prop1 {get;}
}
public interface IBigContext {
int Prop1 {get;}
int Prop2 {get;}
object SomethingElse {get;}
}
}
Так будет только одно место где делается set Prop1 или set Prop2, а весь инпут гет-онли.
это сложно если методы мутируют контекст.
можно сделать большой метод вот таким:
var context = new Context();
context.Prop1 = GetProp1Method(context);
context.Prop2 = GetProp2Method(context);
ActOnFullContext(context);
GetProp1Method(IGetProp1Abstraction input);
GetProp2Method(IGetProp2Abstraction input);
public class Context: IGetProp1Abstraction, IGetProp2Abstraction {
public int Prop1 {get;set;}
public int Prop2 {get;set;}
}
public interface IGetProp1Abstraction {
object SomethingElse {get;}
}
public interface IGetProp2Abstraction {
int Prop1 {get;}
}
public interface IBigContext {
int Prop1 {get;}
int Prop2 {get;}
object SomethingElse {get;}
}
}
Так будет только одно место где делается set Prop1 или set Prop2, а весь инпут гет-онли.
UFO just landed and posted this here
Я с вами согласен: любой радикализм — зло. Как я уже писал в статье, сам грешил тем, про что писал. Тут именно вопрос не применения, а оценки, что есть хорошо, а что есть плохо, что есть необходимое (или почти необходимое) зло. Да иногда приходится так делать в каком-то данном конкретном случае, но это в любом случае говорит об ошибке на более высоком уровне, уровне выбора архитектуры, методики разработки, фреймворка или даже языка.
Иногда комментирую старый код, который потом долго живет в проекте. Причина такая: Иногда сваливается баг и данные на которых его можно воспроизвести, под отладкой я смотрю входные данные в функции, исправляю чтобы все работало и благополучно об этом забываю. Через полгода или позже приходит новый баг с другими данными для воспроизведения, я снова под отладкой смотрю в чем проблема в той же функции и снова исправляю… на первый вариант, написав его один в один, как он был изначально. Поэтому иногда оставляю комментарии:
Было так:
СТАРЫЙ КОД
Так не надо потому что…
UFO just landed and posted this here
Git blame, нормальные ide его умеют
А вот если бы IDE по одному клику тут же показывала историю нужного куска кода, отпала бы необходимость оставлять эту историю в комментариях.
Intellij Idea умеет Git history for selection с контекстного меню, шорткат забиндить, скорее всего, тоже можно.
Visual Studio умеет делать такое на уровне методов. Плюс одна причина держать методы маленькими.
и снова исправляю… на первый вариант, написав его один в один, как он был изначально. Поэтому иногда оставляю комментарии
Хм… после/перед исправлением не помешало бы тест написать, так что в будущем вернуться к ошибочному варианту не получится.
UFO just landed and posted this here
Вот именно, "6 — возвращаем код обратно в состояние 1"
Это и означает — "вернуть ошибочный код не получилось"
UFO just landed and posted this here
После пары таких «залётов» разработчик плюнет и просто оставит в комментариях вариант неправильного кода
У меня в практике именной такой ситуации не случалось, так что может чего-то не понимаю, но разве не может получится, что таких неправильных версий несколько накопится? И что их всех закомментированными держать? Мне всё-таки подход с кратким описанием в духе "делаем так-то потому то и вот эдак делать нельзя иначе..." кажется более удобным и правильным. Тем более, что он лучше масштабируется: если описание разрастается, то его всегда можно перенести куда-то, а в коде оставить ссылку или айди в какой-нибудь джире.
UFO just landed and posted this here
Думаю, если бы у разработчиков, которые так делают были удобные инструменты, позволяющие так не делать, они несомненно пользовались бы ими.
Или нет. Потому что не знают о таких инструментах или привыкли делать как раньше и не хотят ничего менять.
Ну и аргумент с правкой "в блокноте" никогда не понимал. Терпеть не могу работать на ноуте без нормального монитора, клавиатуры и мышки (и привычного окружения). К счастью и не приходится. Другие ситуации, конечно, представить могу, но адекватно о них рассуждать — нет.
git log --pretty=short -u -L 128,135:your-file как раз покажет историю изменения строк 128-135 в файле your-file
Как ни прискорбно, но массу кода в любых проектах можно назвать термином WriteOnly. Ни переписать ни переиспользовать его нельзя, потому как он неотделим от неких скрытых знаний разработчика, написавшего его в некоем контексте прошедшего времени.
Есть проекты целиком writeonly. Кстати расследование логики работы такого кода подчас круче любых детективов.
Есть проекты целиком writeonly. Кстати расследование логики работы такого кода подчас круче любых детективов.
Вот-вот. Буквально на днях сидел разбирался в коде passport-saml, функция SAML.prototype.validatePostResponse меня просто убила
пункты 3 и 4 спорные.
По пункту 3 согласен насчет копи паста бизнес-логики, типа (если статус = 2 или 3 или статус2 = 5) и такая копи-паста. Поддерживать такое нереально.
Еще имеет смысл оборачивать внешние зависимости, аля код типа JsonConvert.Serialize/Deserialize («ну наш же проект всю жизнь будет получать на вход json и только json»)
Но всякую банальную математику или if/else по 3-5 строчек в каждом зачастую только вредит.
Поначалу пока с тебя просят MVP это может выглядеть ок, потом придут требования «а вот это логами обложи», «а сюда телеметрию прикрути», «а сюда нотификейшны добавь» и придется разворачивать обратно.
Про тестирование тоже самое, вроде раздробил и потом у тебя на входе по 10 параметров и пиши эти юнит тесты с пачкой моков. Пока все замокапишь уже забудешь что тестить нужно.
По пункту 4 — согласен если разбивка на методы делается изначально, аля ты пишешь алгоритм сверху вниз:
типа
object DoRequirement1Process(input){
DoStep1()
DoStep2()
DoStep3()
}
void DoStep1() { //TODO: implementation }
void DoStep2() { //TODO: implementation }
void DoStep3() { //TODO: implementation }
и потом каждый раз когда какой-нибудь DoStepX не совсем получается имплементировать как планировалось ты сильно задумываешься и переосмысливаешь всю структуру, а не просто вставляешь костыль и модифицируешь сначала шаги алгоритма, а не вытаскиваешь невлазящий кусок имплементации «чтоб работало».
Придерживаюсь мнения что методы должны быть двух типов: метод-оркестратор (всмысле метод который только вызывает методы других классов), или метод-имплементация (метод который что-то делает и что-то возвращает, но не вызывает ничего сам). Миксовать нельзя, даже если между шагами хочется вставить что-то казалось бы безобидное типа _logger.log() или примитивный foreach внутри которого вызывается 1 метод.
з.ы. работаю сейчас на старом VB.net/VB6 web service с главным классом в районе 80 000 строк и нескольких сторонних классах по 20-40к строк. Приятного мало. Примерно неделю читал его прежде чем представить как же это поменять, спустя 3 месяца уже могу прикинуть в уме примерно где надо что-то поменять, что не есть гуд.
С другой стороны в гринфилд сервисе который пишем над этим веб сервисом один товарищ работал по схеме «напишу ка чтоб работало, потом раскидаю по методам и сервисам». В итоге получаются методы и объекты завязанные под конкретную реализацию, нулевая абстракция и какие-то жесткие утилитарные методы оборачивающие примитивные операции типа арифметики и присвоений через рефлексию или линк экспрешны. Смысл непонятен, поменять реализацию невозможно, вся иерархия этих объектов завязана конкретно на json+rabbitmq+webapi и удачи поменять хоть что-нибудь.
По пункту 3 согласен насчет копи паста бизнес-логики, типа (если статус = 2 или 3 или статус2 = 5) и такая копи-паста. Поддерживать такое нереально.
Еще имеет смысл оборачивать внешние зависимости, аля код типа JsonConvert.Serialize/Deserialize («ну наш же проект всю жизнь будет получать на вход json и только json»)
Но всякую банальную математику или if/else по 3-5 строчек в каждом зачастую только вредит.
Поначалу пока с тебя просят MVP это может выглядеть ок, потом придут требования «а вот это логами обложи», «а сюда телеметрию прикрути», «а сюда нотификейшны добавь» и придется разворачивать обратно.
Про тестирование тоже самое, вроде раздробил и потом у тебя на входе по 10 параметров и пиши эти юнит тесты с пачкой моков. Пока все замокапишь уже забудешь что тестить нужно.
По пункту 4 — согласен если разбивка на методы делается изначально, аля ты пишешь алгоритм сверху вниз:
типа
object DoRequirement1Process(input){
DoStep1()
DoStep2()
DoStep3()
}
void DoStep1() { //TODO: implementation }
void DoStep2() { //TODO: implementation }
void DoStep3() { //TODO: implementation }
и потом каждый раз когда какой-нибудь DoStepX не совсем получается имплементировать как планировалось ты сильно задумываешься и переосмысливаешь всю структуру, а не просто вставляешь костыль и модифицируешь сначала шаги алгоритма, а не вытаскиваешь невлазящий кусок имплементации «чтоб работало».
Придерживаюсь мнения что методы должны быть двух типов: метод-оркестратор (всмысле метод который только вызывает методы других классов), или метод-имплементация (метод который что-то делает и что-то возвращает, но не вызывает ничего сам). Миксовать нельзя, даже если между шагами хочется вставить что-то казалось бы безобидное типа _logger.log() или примитивный foreach внутри которого вызывается 1 метод.
з.ы. работаю сейчас на старом VB.net/VB6 web service с главным классом в районе 80 000 строк и нескольких сторонних классах по 20-40к строк. Приятного мало. Примерно неделю читал его прежде чем представить как же это поменять, спустя 3 месяца уже могу прикинуть в уме примерно где надо что-то поменять, что не есть гуд.
С другой стороны в гринфилд сервисе который пишем над этим веб сервисом один товарищ работал по схеме «напишу ка чтоб работало, потом раскидаю по методам и сервисам». В итоге получаются методы и объекты завязанные под конкретную реализацию, нулевая абстракция и какие-то жесткие утилитарные методы оборачивающие примитивные операции типа арифметики и присвоений через рефлексию или линк экспрешны. Смысл непонятен, поменять реализацию невозможно, вся иерархия этих объектов завязана конкретно на json+rabbitmq+webapi и удачи поменять хоть что-нибудь.
Длинные легаси методы без тестов. У меня есть аргументы против разбиения ради разбиения:
1) засорение истории изменений. Пока метод такой какой он есть, через blame можно сразу увидеть зачем писалась та или иная строка. Если же всё было зарефачено ради компактности, то всё что видно в истории — это последний рефакторинг. Когда кто и зачем написал конкретный блок кода — становится совершенно непонятно. Как вариант можно сначала разобраться и перенести это знание в тесты, но объём усилий уходящий на рефакторинг возрастёт на порядок.
2) Отсутствие юнит тестов. Без них рефакторинг, даже такой безобидный как extract method, превратится в ходьбу по минному полю.
Но это о том, что касается легаси кода. Продолжать так писать, даже при сопровождении этого кода, это преступление.
Например вчера увидел такое:
if (true){ someFunction(); }
Боюсь спрашивать, кто это написал. ;-)
Вспомнилось использование дефолтных названий переменных сначала в Дельфях (ох давно это было), а потом и в Visual Studio
Встречали наверное:
form1;
form2;
…
form14153;
ну или
textBox1
textBox2
…
textBox4234
button1
button2
…
button2354
Sign up to leave a comment.
Уголовный кодекс разработчика