haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба Добрый день! Собственно говоря весь вопрос в теме. Лишь некоторые пояснения требуются. С одной стороны "ревью" - штука замечательная. С другой стороны на своей работе наблюдаю такую картину: начальник отдела часто изучит какую-то новую фичу языка Си++ и начинает её активно проталкивать. Причём, есть ощущение, что делается это не с глубокими пониманием смысла. Одна из моих недавних тем посвящена этому. Сейчас человек настоятельно рекомендует использовать constexpr вместо static const. Например, был код static const auto MY_CONST = 10; Его "следует" заменить на static constexpr auto MY_CONST = 10; При этом не учитывается, что простые типы данных не вызывают конструктора по умолчанию, и такая замена ничего не даёт. Другой пример, у меня был класс с полностью статическими методами. Мне его настоятельно рекомендуют заменить на namespace с функциями. Но не учитывается, что в таком namespace неудобно объявлять переменные (члены класса). В общем вопрос следующий: как у вас на работе обстоит с этим дело? Ведь необходимо, чтобы человек не только умел "красиво" писать код, но при этом код этот транслировался в красивый машинный код. А этот фактор не учитывается, т.к. человек не работает с железом каждый день, а пишет более высокоуровневый код. Более того мне было недавно сказано, что зачем изучать регистры арма и как там всё устроено, когда нужно сначала изучить язык... Я признаю, что некоторых вещей в Си++ не знаю, да и не хочу особо знать, мне больше интересно заниматься железом. Но такое положение дел просто мешает работать: вместо программирования железа и периферии, разбора их нюансов и поведения, начинаешь украшать классы, что ИМХО не так важно. Я не говорю, что код нужно писать левой пяткой через правое ухо. Но то, что я наблюдаю больше похоже на попытку улучшить что-то без понимания вещей. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
Forger 17 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба Ну, если прораб начнет опытного каменщика учить как "правильно" класть кирпичную кладку, то будет закономерно послан на три буквы. Такому прорабу придется крепко задуматься, что нужно и чего не стоит делать, чтобы сохранить должность и уважение рабочих ;) А опытный прораб отлично понимает, что основная задача - построить здание и пройти успешно необходимые проверки прочности и т. п. И потому он не станет соваться со своими "полезными" советами туда, куда не следует Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
mantech 32 5 июня, 2020 Опубликовано 5 июня, 2020 (изменено) · Жалоба 19 минут назад, haker_fox сказал: В общем вопрос следующий: как у вас на работе обстоит с этим дело? У нас очень просто - нужно написать так, чтоб это работало и надежно, какой метод выбирает разработчик - это его дело, и вообще считаю моветоном, когда указывают на чем писать и как это делать. Если мне такое скажут, я этому человеку предложу сделать это самому... ЗЫ. Есть вещи, когда работа идет коллегиально, тогда мы договариваемся, как все это сделать так, чтобы потом "поженить" все части и тут возможны компромиссы... Изменено 5 июня, 2020 пользователем mantech Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
one_eight_seven 3 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба Ваш вопрос ведь не относится к ревью кода. А по факту, если нужен совет, то, о чём вы говорите - это больше относится к разработке стиля кодирования, чем к ревью, как таковому. Предложите создать отдельную задачу по этой теме в вашем трекере задач, И каждое предложение снабдите перечислением плюсов и минусов. А также по каждому спорному случаю можно запускать эксперимент, в котором проверить, так ли существенны улучшения. Ну и, судя по тому, что вы пишете, у вас не какая-то серьёзная продуктовая фирма, а стартап, где можно удобно зацепиться и прокатиться. Ну так и решите для себя - вам это интересно или нет. Если интересно - цепляйтесь, катайтесь, осваивайте новые технологии. Если нет, меняйте начальника. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 5 minutes ago, Forger said: то будет закономерно послан на три буквы. По понятным причинам это не наш метод) 5 minutes ago, Forger said: А опытный прораб отлично понимает, что основная задача - построить здание и пройти успешно необходимые проверки прочности и т. п. Во-во. Он меня учит как программировать красиво. А то, что я полдня разбираюсь почему таймер не работает как хочется или почему возникает такая-то ошибка, не понимает. 6 minutes ago, Forger said: И потому он не станет соваться со своими "полезными" советами туда, куда не следует Эх... 6 minutes ago, mantech said: когда указывают на чем писать и как это делать. Я вот об этом же. Какая разница как писать? Ну, понятно, что ни как попало. 6 minutes ago, mantech said: Есть вещи, когда работа идет коллегиально, тогда мы договариваемся, как все это сделать так, чтобы потом "поженить" все части и тут возможны компромиссы... У нас работа коллегиальная. Но мы пишем каждый свои модули, и то, как они написаны совершенно не влияет на код других коллег... 3 minutes ago, one_eight_seven said: о чём вы говорите - это больше относится к разработке стиля кодирования, чем к ревью, как таковому. Возможно. Наш начальник считает, что ревью это и стиль. Причём на стиль я бы сказал у него больший акцент. Что заставляет задуматься. 4 minutes ago, one_eight_seven said: Ну и, судя по тому, что вы пишете, у вас не какая-то серьёзная продуктовая фирма, Не стартап. Фирме уже более 25 лет. И давно и даже успешно выпускаем продукцию. Но эти придирки к коду... за...ют уже... 4 minutes ago, one_eight_seven said: Если нет, меняйте начальника. Чувствую, что такой начальник один - я сам) Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
Corvus 1 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба Самодурство начальника не имеет к ревью никакого отношения. Ревью это не сдача проекта заказчику. А конструктивный диалог между специалистами, который позволяет улучшить конечный результат. Если единственный ревьюер - ваш руководитель, который не воспринимает никаких доводов и не хочет аргументировать свою позицию, то вариантов два: 1. Смириться 2. Сменить работу. 6 minutes ago, mantech said: У нас очень просто - нужно написать так, чтоб это работало и надежно Надежно работать может и такой код, в котором не разберётся никто, кроме автора. А это критично для больших долгоиграющих проектов. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 2 minutes ago, Corvus said: Если единственный ревьюер - ваш руководитель, По факту так. На самом деле в отделе 2 толковых программиста, но для ПК. Начальник, тоже вроде хорошо программирует, или кажется, что хорошо, но он как-то в железо не вникает. Я в отделе один, кто может быт и некрасиво программирует, но лучше всех знает железо (кортексы, ацп, шины и т.п.). Тут, без гордныни. Так и есть. Ибо все ко мне идут с вопросами, почему хардфолт, почему i2c не передаёт данные, почему spi не работает и т.п. Вот я и думаю, какого лешего меня грузить второстепенными вопросвми? 4 minutes ago, Corvus said: 1. Смириться Пока этот вариант. Но с капелькой гнева. 5 minutes ago, Corvus said: А это критично для больших долгоиграющих проектов. Ну в моём случае придирки странные: ты завёл лишний класс, ты завёл целый файл ради 20 строк работы с таймером... А проект в стадии развития, и не учитывается, что сегодня этот класс работает только с таймером, а завтра туда может добавиться ПДП... Тем более, что самое интересное, нет отсылок к стандартам, которые это запрещают делать. В т.ч. и к внутреннему стандарту. Чаще отсылки к знаменитому Майерсу.. 7 minutes ago, Corvus said: Самодурство начальника не имеет к ревью никакого отношения. ИМХО, начальник вообще редко должен вмешиваться в работу специалистов, уровень которых подтверждён зарплатой... Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
one_eight_seven 3 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 2 minutes ago, haker_fox said: Не стартап. Фирме уже более 25 лет. И давно и даже успешно выпускаем продукцию. Но эти придирки к коду... за...ют уже... Тогда, возможно, вы чего-то не видите. В устоявшихся компаниях, как правило, не бегут за самыми новыми технологиями. Т.е. не увидишь сейчас в крупных устоявшихся компаниях C++20, даже C++17, 14 уже всё активнее появляется, но пока больше всего 11 или вообще 98. Это имеет минусы - нечем привлекать молодых и активных, но есть очень много кода, который написан уже в тех версиях языка, с идиомами и технологиями тех языков, со стилем, разработанным под них. Упор начальника на стиль - это вещь правильная и объяснимая. Удивительно, при этом, что это он туда тащит новые фунции, а не работники. Что-то у вас не так, если работникам некогда интересоваться новинками. Обычно начильник, наоборот, является адвокатом старого стиля, и в той табличке, которую я вам советовал, в столбце с минусами у каждой новой технологии сильнейшим грузом лежит: "У нас гигабайты кода, написанные без этого. Какие проблемы это нововведение решает?". И вот это тоже проблема, которую решает стиль кодирования. Есть вещи, над которыми думать нужно, а есть те, над которыми не нужно. И вопрос - где ставить фигурные скобки, где использовать передачу по ссылке, а где передачу по значению, как возвращать ошибки - через возвращаемое значение или бросать исключения - это то, что диктует стиль, и то, о чём думать не надо, то, что у всех в компании должно быть одинаково. Я объясню. Когда все придерживаются одинакового стиля, резко уменьшается количество багов. Когда один программист смотрит на код другого - он сразу понимает, как это использовать (это, конечно, со временем приходит и со зрелостью стиля), не попадает в ловушку неправильной интерпретации. Если у кого-то возникает ошибка, которую он не может найти, то, со временем, всё чаще возникает ситуация, когда кто-то эту проблему уже решил, и он, видя знакомый код, сразу скажет в чём было дело. Так что, корпоративный стиль кодирования - это вещь великая. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
mantech 32 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 7 минут назад, one_eight_seven сказал: В устоявшихся компаниях, как правило, не бегут за самыми новыми технологиями. Вот никогда этого не понимал, зачем применять что-то другое (новое, модное и т.п.) если на том, что сейчас используется можно сделать то же самое? Ну вот зачем? Если возможностей не хватает - тогда другое дело, но в большинстве своем, как правило, всего хватает... 10 минут назад, one_eight_seven сказал: Обычно начильник, наоборот, является адвокатом старого стиля, и в той табличке, которую я вам советовал, в столбце с минусами у каждой новой технологии сильнейшим грузом лежит: "У нас гигабайты кода, написанные без этого. Какие проблемы это нововведение решает?". +1 Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 14 minutes ago, one_eight_seven said: не бегут за самыми новыми технологиями. У нас не совсем так... И представляете, как иногда трудно железячнику? Тебе и шины, и АЦП, и ЦАП и архитектуру МК и ещё кучу вещей нужно изучать, освежать в памяти. А тут ещё надо, оказывается, constexpr вместо const применять.. 14 minutes ago, one_eight_seven said: но есть очень много кода, который написан уже в тех версиях языка, А это есть. Но всё равно для новых проектов пишется код с новыми реалиями... 14 minutes ago, one_eight_seven said: Что-то у вас не так, если работникам некогда интересоваться новинками. Смотря каким новинками. Я вот недавно занимался тем, что читал про архитектуру ARMv6-V, с которой предстояло работать после долгой работы с более старшими армами. Изучал ассемблер, в частности, чтобы более оптимально писать программы, если понадобится. Мне не интересно гнаться за новинками чисто языка. Более того, настораживает непонимание начальника ограничений архитектуры. Несколько лет назад по дурости я затащил std::map, std::string и new в проект на STM32F051 с 8 кБ ОЗУ, поддавшись на соблазн того, что предоставляют эти штуковины. В итоге, всё было выкинуто где возможно, и заменено на более легковесные вещи. А кто мне это подсказал? О чём это говорит? О том, что человек видит верхушку, но не видит низов. 14 minutes ago, one_eight_seven said: передачу по ссылке, а где передачу по значению, как возвращать ошибки - через возвращаемое значение или бросать исключения - это то, что диктует стиль А такого у нас в регламенте нет... 14 minutes ago, one_eight_seven said: Так что, корпоративный стиль кодирования - это вещь великая. С этим согласен. Но в стиле всего, что начальник требует нет. Ещё никто стиль не поправил. Более того, вчера в ревью он указал коллеге, что не нужно инициализировать нулём стековые переменные типа size_t. Типа они автоматом инициализируются нулём. Это уже как-то совсем. 5 minutes ago, mantech said: +1 а тож! Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
mantech 32 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 1 минуту назад, haker_fox сказал: что не нужно инициализировать нулём стековые переменные типа size_t. Типа они автоматом инициализируются нулём. Из вас ему еще никто не предложил сесть и написать все самому? Эдак он и алгоритмы там "свои" модные предлагать начнет... Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
one_eight_seven 3 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 1 minute ago, mantech said: Вот никогда этого не понимал, зачем применять что-то другое (новое, модное и т.п.) если на том, что сейчас используется можно сделать то же самое? Ну вот зачем? Затем, что часто это, действительно, удобнее, понятнее, лаконичнее. Пример. Передача (присваивание) по позиции и по имени. Сначала появилось присваивание по позиции, и как только появилось и стабилизировалось присваивание по имени, я стал использовать её, и мой код стал гораздо понятнее, и всем это советую. Если вы знакомы с verilog, SystemVerilog, думаю, вам это известно - передача в модули уже давно подавляющим большинством инженеров используется только по имени. То же самое мождно видеть со структурами в C, C++ там всё чаще используется присваивание по имени поля. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба Just now, mantech said: Из вас ему еще никто не предложил сесть и написать все самому? У нас народ предпочитает рот не раскрывать. А меня уже пару раз вызывали в отдельный кабинет "для разговора". Типа я много говорю. Теперь предпочитаю помалкивать, ибо всё равно ничего не изменить. 1 minute ago, mantech said: Эдак он и алгоритмы там "свои" модные предлагать начнет... Мне кажется, ограниченные люди просто цеплюяются к тому, что понимают. Видят бревно на обочине- не порядок, некрасиво, надо убрать. Убрали. А бревно лежало для нового столба, который завтра будут менять. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
Corvus 1 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 5 minutes ago, haker_fox said: ИМХО, начальник вообще редко должен вмешиваться в работу специалистов, уровень которых подтверждён зарплатой... Тогда это не начальник, а заказчик. Или внутренний заказчик (вроде продакт менеджера, продакт оунера и т.д.) А то получается, что тренер не должен вмешиваться в игру футболистов. Абсурд какой-то. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
haker_fox 59 5 июня, 2020 Опубликовано 5 июня, 2020 · Жалоба 1 minute ago, one_eight_seven said: и всем это советую. Советуете. А начальники редко советуют. Они привыкли к тому, что совет = приказ. И если возразишь, некоторые ещё на тебя обижаются. Или унижают твои знания, как ненужные. Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться