LINUX.ORG.RU

Ревью кода или психология мидла

 ,


3

8

Всем привет!

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

И любит он делать херовый код (плохой нейминг, непонятные и ненужные абстракции, каша в логике). Если пнуть, то обычно исправляет. Но я уже заманался его пинать, одни и те же ошибки в каждом МР. Уволить?! Как говорит начальство — не можем, бюджет не позволяет платить больше кому-то, а найти нового человека сейчас очень сложно.

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

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

Что обычно делают в таких ситуациях? Писать что все фигня, давай по новой и закрывать мерж? Или есть какие-то техники? Подскажите или тыкните что почитать?

★★★★

Последнее исправление: maxcom (всего исправлений: 1)

Вариант 1 - уволиться и искать работу с менее стрессовым окружением.

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

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

Какой вариант тебе ближе - смотри сам.

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

vbr ★★★
()

Почитал тему, добавлю ещё один вариант.

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

  2. Внести в проект максимальную автоматизацию проверки качества. Проверять форматирование соответствующими инструментами, проверять качество кода соответствующими инструментами. Интегрировать цикл сборки и эти инструменты так, чтобы просмотр кода даже не начинался, пока все проверки не пройдут. Для разных языков есть разного уровня инструменты, совет не очень универсальный, к сожалению, но для той же Java Checkstyle, к примеру, даёт очень широкие возможности по проверке кода.

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

vbr ★★★
()
Последнее исправление: vbr (всего исправлений: 1)
Ответ на: комментарий от Lrrr

У нас DDD и чистый код, абстракции больше определяются через DDD, а чистота кода соответственно правилами CleanCode

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

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

vbr ★★★
()
Последнее исправление: vbr (всего исправлений: 1)
Ответ на: комментарий от olelookoe

нейминг полюбас под НДА не попадает

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

Nervous ★★★★★
()
Последнее исправление: Nervous (всего исправлений: 1)

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

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

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

AKonia ★★
()

Ну так это вполне норм ситуёвина.

Был я начальником группы на второй работе, делали разное оборудования для РЖД и прочего. Не то, чтобы нужно было по ГОСТам чертить, как в военке, но все-таки че-то среднее. А меня учили как в военке… Это когда военпред дерёт во все дыры за не тот размер шрифта или толщину линии.

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

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

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

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

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

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

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

Zhbert ★★★★★
()
Ответ на: комментарий от AKonia

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

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

hateyoufeel ★★★★★
()
Ответ на: комментарий от AntonyRF

Уходишь на больничный, через две недели там ахтунг

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

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

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

Zhbert ★★★★★
()
Ответ на: комментарий от Zhbert

место в забугорной компании в итоге тоже конструктором

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

Инженеры, мне кажется, менее мобильная категория, чем программисты.

the_real_kinik ★★
()
Ответ на: комментарий от stave

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

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

Zhbert ★★★★★
()
Ответ на: комментарий от the_real_kinik

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

Zhbert ★★★★★
()
Ответ на: комментарий от hateyoufeel

И даже когда не ложных.

  • Вам стало сложнее писать, когда ограничили цикломатическую сложность и длину процедуры?
  • Почти нет. Просто теперь после того, как напишу процедуру, я делаю все тела циклов и условий отдельными процедурами, а потом то, что осталось, делю на куски по 99 строк.

И код выглядит примерно так:

int foo(int x, int y)
{
  int a, b;
  for (int i = 0; i<=limit; i++) {
    foo_loop1(x, y, i, &a, &b);
  }
  if (some_condition(a, b)) { 
    foo_if1_true(x, y, &a, &b);
  } else { 
    foo_if1_false(x, y, &a, &b); 
  };
  ... // строка 99
  return foo_cont1(x, y, a, b);
}

Линтер счастлив.

monk ★★★★★
()
Последнее исправление: monk (всего исправлений: 1)

У меня проект который лепило 10 человек и там солянка стилей, он как пестрое одеяло, но работает… Ты же тормозишь разработку. Я бы тебя уволил: перфекционисты невыносимы.

anonymous
()
Ответ на: комментарий от hateyoufeel

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

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

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

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

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

Вот так работают хорошие команды со статическими анализаторами.

Aber ★★★★★
()
Последнее исправление: Aber (всего исправлений: 2)
Ответ на: комментарий от Aber

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

Так можно ненароком и резак Шири создать.

monk ★★★★★
()
Ответ на: комментарий от areful

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

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

Mobutu_Sese_Seko
()
Последнее исправление: Mobutu_Sese_Seko (всего исправлений: 2)
Ответ на: комментарий от anonymous

для этого микросервисы сущ. Каждый лепит под себя и чудо - ОНО работает … И отлаживать на порядок проще, чем говно-монолит - даже если у каждого свой говно-стиль.

ivanich10
()
Последнее исправление: ivanich10 (всего исправлений: 1)
Ответ на: комментарий от ivanich10

для этого микросервисы сущ. Каждый лепит под себя и чудо - ОНО работает

Иногда работают, иногда нет. Но ведь для этого есть кубер, чтобы их перезапускать, не так ли? %)

Nervous ★★★★★
()

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

demidrol ★★★★★
()

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

Obezyan
()

и пишешь куски кода для того чтобы он порефакторил правильно

А не надо писать куски кода. Один раз показал, дальше достаточно оставить отписку типа кодстайл, const, naming, нарушение srp и всё, пусть переписывает хоть миллион раз пока не сделает правильно. А чтобы каждый раз не тратить время на ревью одного и того же их надо заставлять дробить, если надо хоть на однострочные. Когда мозгов прибавится можно постепенно увеличивать.

anonymous
()
Ответ на: комментарий от Aber

Новые правила статического анализа вносятся аккуратно.

Я про линтеры писал, а не про статический анализ.

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

hateyoufeel ★★★★★
()
Ответ на: комментарий от Obezyan

подчинение т.е. формально мы на одном уровне

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

ivanich10
()
Ответ на: комментарий от goingUp

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

пуркуа на фейхуа?

зы: чел своим кодом повышает защищённость своего низкооплачиваемого рабочего места а ты мешаешь - не надо так

qulinxao3
()
Ответ на: комментарий от AntonyRF

у вас(рабкодателя) - как обычно «вот тебе 'копьё'йка сделай на сотнинефти» - наречекрякать всегда(так кажется ибо не требует отсутствющих(всегда)бюджетов - а требует того что и так в избытке у руководоства-вдохновить на субботник) дешевле - в этом(вашем) просторанство-времени месте в моде аж DDD c магией CleanCode - чем реально построить процессы по той или иной ой-методологии - и практиковать отрабатывая все несоответствия между теорией и её приложением

у вас проблема на уровня 2 выше чем тебе кажется

qulinxao3
()

Всем привет, в общем есть некий абстрактный крупный проект новостного сайта, делаю админку для него, в проекте DDD + CQRS.

rust

middle

Сдается мне автор в неадеквате

rtxtxtrx
()
Ответ на: комментарий от qulinxao3

Он взял Rust, начал на нем писать в Java-стиле… Казалось бы что могло пойти не так… Короче запорол проект. Умение руководить сводится к нахождению компромиссов, а тут у нас ярый сторонник ереси, который сам лбом об стену бьется и пытается другим трахать мозг своими представлениями о совершенстве… обмазываться тепленьким, свежевысратым дерьмецом… Еще стремнее, что какой-то аметист разрешил крупный проект писать на Rust’е… вебню… языке не_предназначенном для этого… Я бы на правах антикризесной менеджерки уволил бы автора, его коллегу, того кто инициировал этот идиотизм и переписал проект на тепленьком, мягоньком питоне… Ну серьезно: Node.js/Python для микросервисов на худой конец Go

rtxtxtrx
()
Последнее исправление: rtxtxtrx (всего исправлений: 3)
Ответ на: комментарий от anonymous

Сначала нашёл на https://lesswrong.ru/w статьи Скотта Александера (там как раз много чего в стиле этого рассказа, например, Предотвращение Ашибок). Потом начал искать, что ещё есть от этого автора. Англоязычного очень много на https://slatestarcodex.com/.

monk ★★★★★
()

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

Кесарю кесарево(с).

sambo ★★
()

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

пытается затягивать тимлидов на обсуждение ревью

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

anonymous
()
Ответ на: комментарий от sambo

Палка о двух концах, знаешь ли…

С одной стороны, да - ТС добровольно берет на себя больше ответственности (таких всё меньше, увы). И бизнес не его, да, и 2.7бись бы оно конём, и бросить бы на самотёк - работает же в краткосроке, НО

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

И, тут выше писали, самое правильное - дать обосраться этим деятелям, но это долго и дорого. И потом конюшни еще чистить…

aol ★★★★★
()
Ответ на: комментарий от aol

С другой стороны, рано или (скорее всего) поздно бизнес заметит, что «характеристики отдела разработки падают»

Или бизнес заметит душнилу, который тормозит процесс на основе лично своего «чувства прекрасного» :)

anonymous
()