Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
J>Как лучше просить что-то добавить? J>"Would you, please, ..." J>"Will you, please, ..." J>"Could you, please, ..." J>"How about to .. " J>"Is this ... done?"
Давно читал в англоязычном блоге, помню плохо, а сейчас не смог найти. Но вроде некоторые обороты, которые в обычной жизни могут вполне использоваться, в корпоративной лексике имеют другой смысл. Например "could you" в контексте ревью может читаться ими как агрессивное "способен ли ты" с уклоном в "сомневаюсь" а "Is this ... done?" вопрос уровня "наставник ученику", т.е. корректен если ты какой лид, или сеньёр ревьювящий за джуном. А для равных между собой он используется только когда вы уже враги которые друг друга валят при первой возможности. Злорадное "вот ты и попался! вот тут нот дан!".
Короче это надо спрашивать не тут, а на форуме нейтивных нейтивов, живущих офисной жизнью.
Здравствуйте, IT, Вы писали:
G>>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
IT>Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".
Не, там нормально всё было. Грубо говоря, некий построитель отчетов (абстрактный класс), который разные виды отчетов генерит (csv, pdf, word и т.д.). И просто в эту иерархию потом засунули совсем другой класс-потомок, который своим чуждым поведением нарушил стройную картину "абстрактной логики", внеся в тот слой явную обработку вариаций этого "особого поведения" подкласса-бастарда. А это явное нарушение LSP. В итоге просто мы поняли, что этот класс-потомок вообще не относится к данной иерархии и вынесли его в отдельное место. Иерархия после этого сказала нам спасибо и восстановила принципы LSP и DIP.
Здравствуйте, #John, Вы писали:
J>Здравствуйте, J>как вежливо делать замечания по коду, желательно с примерами на англ.
Обычно помогает если хранится история предыдущих рецензий, например, в виде смерженных PR'ов в гите. Там обычно видно, как принято общаться в процессе обсуждения кода. Ну это вдобавок к любым гайдлайнам, принятым в организации (если есть).
Здравствуйте, Osaka, Вы писали:
scf>>Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy? O>Отвратительное лицемерие. Вводит в опасные заблуждения. Сторона, сдающая работу, считает что с ней хотят посоветоваться и начинает приводить аргументы что надо так как сделано, а принимающая (работодатель) — ни слова не говоря заносит работника в чёрный список "не выполняющих прямые распоряжения".
У вас какое-то специфическое место работы. Никогда не видел программистов, которыми управляют через прямые распоряжения, это неконструктивно. "заставить сделать хорошо" — это важный результат процесса ревью, но далеко не единственный. Не менее важно обучение (в том числе взаимное) и распространение знаний.
scf>Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
Я вот тоже чаще всего в такой форме пишу. Кстати, кроме простой вежливости, еще один плюс — меня это немного избавляет от страха что я сам просто протупил и не понял, что и зачем написал автор и зря на него наехал.
Здравствуйте, #John, Вы писали:
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень? J>"The code will be look much better if ..., what do you think?"
А официально принятые в рамках организации или проекта гайдлайны по коду есть?
Здравствуйте, #John, Вы писали:
J>Здравствуйте, J>как вежливо делать замечания по коду, желательно с примерами на англ.
J>Как лучше просить что-то добавить? J>"Would you, please, ..." J>"Will you, please, ..." J>"Could you, please, ..." J>"How about to .. " J>"Is this ... done?"
J>...
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
J>"The code will be look much better if ..., what do you think?"
You are awesome, but this is mthfk bullshit.
А если серьезно, ты не заколебешся на каждое замечание писать polite тексты? Не лучше ли на дейли всех хвалить за их работу, и говорить что код ревью — это совместная работа и т.д., что все молодцы, что замечания на код ревью не призваны никого оскорбить или наругать. А в замечаниях просто и лаконично писать по делу?
Здравствуйте, #John, Вы писали:
J>как вежливо делать замечания по коду
1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания
2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Указать конкретные проблемы в чем "не очень", если они требуют переписывания.
Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
Здравствуйте, fmiracle, Вы писали:
J>>как вежливо делать замечания по коду
F>1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания F>2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.
J>>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
F>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
Здравствуйте, gyraboo, Вы писали:
J>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень? F>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
G>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
0. Я полностью тут с тобой согласен
при этом
1. обращение к справочнику проблем и практик — это тоже вариант обоснования же
2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же
Здравствуйте, fmiracle, Вы писали:
J>>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень? F>>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
G>>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
F>0. Я полностью тут с тобой согласен F>при этом F>1. обращение к справочнику проблем и практик — это тоже вариант обоснования же F>2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же
SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Можно спросить "почему ты решил именно скопировать эту часть кода, а не выделить функцию? Создал таск чтобы разобраться." вместо "з??;л уже копипастить! деклайн!"
Здравствуйте, #John, Вы писали:
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Напомнило мне как в почтовые письма автоматически вставляется текст, вроде с уважением ля ля ля. Хотя лично я такое не люблю так как по сути это ненужная для прочтения информация.
Здравствуйте, #John, Вы писали:
J>Здравствуйте, J>как вежливо делать замечания по коду, желательно с примерами на англ.
J>Как лучше просить что-то добавить? J>"Would you, please, ..." J>"Will you, please, ..." J>"Could you, please, ..." J>"How about to .. " J>"Is this ... done?"
J>...
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
J>"The code will be look much better if ..., what do you think?"
Можно посмотреть кучу примеров в больших проектах с открытым исходным кодом.
Например Qt: https://codereview.qt-project.org/c/qt/qtbase/+/343993
> I'd like to have some opinions on this design decision. Should we require the use of
Здравствуйте, gyraboo, Вы писали:
G>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
IT>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID
Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
Здравствуйте, gyraboo, Вы писали:
IT>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID G>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
Не после применения, а до.
Возьмём тот же LSP, который гласит:
объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы
Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
IT>>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID G>>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
IT>Не после применения, а до. IT>Возьмём тот же LSP, который гласит:
IT>
объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы
IT>Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.
Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
А насчет сложных иерархий — согласен. Ну на это уже есть и этой бочке затычка другие принципы: заменяйте наследование композицией, используйте магическое число 7 (а лучше меньше, как ограничение кратковременной памяти), KISS.
Здравствуйте, scf, Вы писали:
scf>Здравствуйте, #John, Вы писали:
J>>Здравствуйте, J>>как вежливо делать замечания по коду, желательно с примерами на англ.
scf>Есть хорошая статья на SO https://stackoverflow.blog/2019/09/30/how-to-make-good-code-reviews-better/
scf>Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
Примерно так и стараюсь делать. Иногда пишу что-то типа 'looks like breaking change (details here) please explain' или 'seems to be performance issue (details here) please explain'. Крайне редко пишу жестко (если это не первая попытка ревью, а человек не понимает).
Здравствуйте, gyraboo, Вы писали:
G>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, gyraboo, Вы писали:
G>>>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации. IT>>Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца". G>Не, там нормально всё было. Грубо говоря, некий построитель отчетов (абстрактный класс), который разные виды отчетов генерит (csv, pdf, word и т.д.). И просто в эту иерархию потом засунули совсем другой класс-потомок, который своим чуждым поведением нарушил стройную картину "абстрактной логики", внеся в тот слой явную обработку вариаций этого "особого поведения" подкласса-бастарда. А это явное нарушение LSP. В итоге просто мы поняли, что этот класс-потомок вообще не относится к данной иерархии и вынесли его в отдельное место. Иерархия после этого сказала нам спасибо и восстановила принципы LSP и DIP.
Нет, ты всё-таки не понимаешь. Передезайнив иерархию, вы не восстановили принцип, а исключили необходимость его применения. Применять его нужно было когда вы засунули совсем другой класс-потомок куда не нужно. Принцип Лисковой предписывает так не делать. Вы же поступив грамотно, полностью исключили такую возможность. Именно об этом я и говорю.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, #John, Вы писали:
J>Здравствуйте, J>как вежливо делать замечания по коду, желательно с примерами на англ.
J>Как лучше просить что-то добавить? J>"Would you, please, ..." J>"Will you, please, ..." J>"Could you, please, ..." J>"How about to .. " J>"Is this ... done?"
J>...
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
J>"The code will be look much better if ..., what do you think?"
1. Please в английском запятыми не выделяется.
2. Места для комментариев мало, экивоки излишни.
3. Сомнительный код, как правило, можно подсветить, поэтому this code излишне.
4. WTF? и BS! -- неконструктивное хамство. Поэтому оценки опускаем и сразу переходим к делу, для вежливости и придания формы обсуждения предложения лучше заверщать знаком вопроса.
Здравствуйте, #John, Вы писали:
J>как вежливо делать замечания по коду, желательно с примерами на англ.
Прежде всего помни, что вежливое посылание на юг- все ещё посылание на юг.
Если действительно проблема, обьясни, почему так, предложи варианты, обсуди в чате. Придирательства, крючкотворство и вкусовщина только бесят.
Проблема может быть, что фактически неправильно, или нарушен принцип лискова (но не нужно называть это), неэффективно. Если ничего этого нет, если твоя вкусовщина возмущена- подтверди PR.
Здравствуйте, IT, Вы писали:
G>>>>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации. IT>>>Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца". G>>Не, там нормально всё было. Грубо говоря, некий построитель отчетов (абстрактный класс), который разные виды отчетов генерит (csv, pdf, word и т.д.). И просто в эту иерархию потом засунули совсем другой класс-потомок, который своим чуждым поведением нарушил стройную картину "абстрактной логики", внеся в тот слой явную обработку вариаций этого "особого поведения" подкласса-бастарда. А это явное нарушение LSP. В итоге просто мы поняли, что этот класс-потомок вообще не относится к данной иерархии и вынесли его в отдельное место. Иерархия после этого сказала нам спасибо и восстановила принципы LSP и DIP.
IT> Нет, ты всё-таки не понимаешь. Передезайнив иерархию, вы не восстановили принцип, а исключили необходимость его применения. Применять его нужно было когда вы засунули совсем другой класс-потомок куда не нужно. Принцип Лисковой предписывает так не делать. Вы же поступив грамотно, полностью исключили такую возможность. Именно об этом я и говорю.
Я всё равно не понял мысль.
Вот смотри: была иерархия классов, не имеющая нарушений в LSP и DIP.
В одном из следующих коммитов внесли новый подкласс, который стал нарушать LSP.
В результате подкласс вынесли в отдельную иерархию, восстановив LSP.
Исходную иерархию мы не передизайнивали, а оставили как есть, а создали новую, для нового вида поведения, не укладывающегося в старую абстрактную логику.
При этом нельзя сказать, что мы "исключили принцип LSP". Мы его как раз применили, чтобы защитить иерархию от его нарушений. Т.е. исключив возможность нарушения принципа LSP, мы как раз-таки соблюли принцип LSP. Ведь теперь и старая иерархия осталась соблюдать LSP, и новая иерархия тоже соблюдает LSP.
scf>Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
Отвратительное лицемерие. Вводит в опасные заблуждения. Сторона, сдающая работу, считает что с ней хотят посоветоваться и начинает приводить аргументы что надо так как сделано, а принимающая (работодатель) — ни слова не говоря заносит работника в чёрный список "не выполняющих прямые распоряжения".
Здравствуйте, gyraboo, Вы писали:
G>Я всё равно не понял мысль.
Ну хорошо. Давай пустимся в крайности. Допустим вы изначально решали проблему без всякий иерархий каких бы то ни было классов. Т.е. нет объектов, нет возможности нарушить LSP. Правильно?
G>Вот смотри: была иерархия классов, не имеющая нарушений в LSP и DIP.
Не имеющая нарушений потому что вы внимательно за этим следили или же вы не имели возможности ничего нарушить?
G>В одном из следующих коммитов внесли новый подкласс, который стал нарушать LSP.
Похоже следили, но не уследили.
G>В результате подкласс вынесли в отдельную иерархию, восстановив LSP. G>Исходную иерархию мы не передизайнивали, а оставили как есть, а создали новую, для нового вида поведения, не укладывающегося в старую абстрактную логику. G>При этом нельзя сказать, что мы "исключили принцип LSP". Мы его как раз применили, чтобы защитить иерархию от его нарушений. Т.е. исключив возможность нарушения принципа LSP, мы как раз-таки соблюли принцип LSP. Ведь теперь и старая иерархия осталась соблюдать LSP, и новая иерархия тоже соблюдает LSP.
Понятно. Т.е. через пару лет на ваше место придёт Вася Петин и с лёгкостью всё поломает и нарушит. Фактически главную проблему вы не решили, вы просто грамотно в соответствии с канонами её обошли.
Я поначалу подумал, что вы сделали всё правильно и передезайнили исходную иерархию, исключив таким образом любую возможность нарушить любые принципы
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>Я всё равно не понял мысль.
IT>Ну хорошо. Давай пустимся в крайности. Допустим вы изначально решали проблему без всякий иерархий каких бы то ни было классов. Т.е. нет объектов, нет возможности нарушить LSP. Правильно?
Я кажется понимаю, к чему ты клонишь. К DLS (Domain Specific Language)? Т.е. мета-языку, на котором можно реализовывать специфичные бизнесовые, и нельзя ошибиться, т.к. сам синтаксис, семантика и интерпретатор языка не позволяют неправильно реализовать задачу?
Или ты имеешь ввиду, что можно каким-то образом разрабатывать энтерпрайзное ПО на ЯП общего назначения, но каким-то магическим образом заложить в него ограничения, что никакой Петя или Вася при всем желании не испортят изначально заложенную архитектуру?
G>>Вот смотри: была иерархия классов, не имеющая нарушений в LSP и DIP.
IT>Не имеющая нарушений потому что вы внимательно за этим следили или же вы не имели возможности ничего нарушить?
Ну да, за этим нужно постоянно следить и культивировать, иначе очень быстро всё идёт в разнос, и бывает даже разрабы, которые знают принципы Clean code, начинают девелопить "грязно".
G>>В одном из следующих коммитов внесли новый подкласс, который стал нарушать LSP.
IT>Похоже следили, но не уследили.
Ну да, именно так. На этапе код ревью это отлавливается. Ну ещё вариант — добавить автоматические средства, я иногда их использую для проверки самых общих архитектурных правил, но ручную проверку это не исключает.
G>>В результате подкласс вынесли в отдельную иерархию, восстановив LSP. G>>Исходную иерархию мы не передизайнивали, а оставили как есть, а создали новую, для нового вида поведения, не укладывающегося в старую абстрактную логику. G>>При этом нельзя сказать, что мы "исключили принцип LSP". Мы его как раз применили, чтобы защитить иерархию от его нарушений. Т.е. исключив возможность нарушения принципа LSP, мы как раз-таки соблюли принцип LSP. Ведь теперь и старая иерархия осталась соблюдать LSP, и новая иерархия тоже соблюдает LSP.
IT>Понятно. Т.е. через пару лет на ваше место придёт Вася Петин и с лёгкостью всё поломает и нарушит. Фактически главную проблему вы не решили, вы просто грамотно в соответствии с канонами её обошли. IT>Я поначалу подумал, что вы сделали всё правильно и передезайнили исходную иерархию, исключив таким образом любую возможность нарушить любые принципы
Ну давай, делись волшебной таблеткой! Как эту проблему с необходимостью ручного код ревью можно решить? ))
Здравствуйте, gyraboo, Вы писали:
G>Ну давай, делись волшебной таблеткой!
Какая ещё таблетка? Чтобы выписать вам пилюлю нужно как минимум подробно разобрать ваш код.
G>Как эту проблему с необходимостью ручного код ревью можно решить? ))
Вот тут я не понял. Как необходимость ручного ревью влияет на дизайн приложения?
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>Ну давай, делись волшебной таблеткой!
IT>Какая ещё таблетка? Чтобы выписать вам пилюлю нужно как минимум подробно разобрать ваш код.
Ну ты же говоришь о том, что можно как-то спроектировать иерархию классов так, чтобы потом ни один суппортер не вносил в неё изменений, нарушающих принципы SOLID, даже если человек не очень хорошо их понимает? Т.е. иерархия каким-то образом запрещает менять её неправильно?
G>>Как эту проблему с необходимостью ручного код ревью можно решить? ))
IT>Вот тут я не понял. Как необходимость ручного ревью влияет на дизайн приложения?
А ты имеешь ввиду, что можно так спроектировать классы изначально, что потом она сама собой будет разрастаться только в "правильном" направлении? И это "правильное" направление (соблюдение SOLID и прочие принципы) как-бы настолько сильно и мощно заложено в изначальной архитектуре, что даже джуны и мидлы, которые пока не очень освоили SOLID, будут при доработке системы развивать её только в "правильном" направлении, без необходимости код ревью их кода?
Здравствуйте, gyraboo, Вы писали:
IT>>Какая ещё таблетка? Чтобы выписать вам пилюлю нужно как минимум подробно разобрать ваш код. G>Ну ты же говоришь о том, что можно как-то спроектировать иерархию классов так, чтобы потом ни один суппортер не вносил в неё изменений, нарушающих принципы SOLID, даже если человек не очень хорошо их понимает? Т.е. иерархия каким-то образом запрещает менять её неправильно?
Примерно так. Но опять вопрос — почему обязательно иерархия и обязательно классов?
G>А ты имеешь ввиду, что можно так спроектировать классы изначально, что потом она сама собой будет разрастаться только в "правильном" направлении? И это "правильное" направление (соблюдение SOLID и прочие принципы) как-бы настолько сильно и мощно заложено в изначальной архитектуре, что даже джуны и мидлы, которые пока не очень освоили SOLID, будут при доработке системы развивать её только в "правильном" направлении, без необходимости код ревью их кода?
К этому нужно стремиться. В чём проблема?
Если вернуться к моему изначальному комментарию, то я лишь утверждал, что необходимость следования данным принципам уже свидетельствует о проблемах в коде. Твой пример это только подтвердил. А как это решать — без понятия. У каждой конкретной проблемы в каждом конкретном случае своё конкретное решение.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
IT>>>Какая ещё таблетка? Чтобы выписать вам пилюлю нужно как минимум подробно разобрать ваш код. G>>Ну ты же говоришь о том, что можно как-то спроектировать иерархию классов так, чтобы потом ни один суппортер не вносил в неё изменений, нарушающих принципы SOLID, даже если человек не очень хорошо их понимает? Т.е. иерархия каким-то образом запрещает менять её неправильно?
IT>Примерно так. Но опять вопрос — почему обязательно иерархия и обязательно классов?
Потому что энтерпрайз в джаве пишут в основном на ООП. 90% разработчиков учили в институте ООП и владеют именно ООП-техниками. Кроме того, именно для ООП (или для анемичной модели, но она тоже построена на классах) есть много проверенных решений типичных проблем энтерпрайза.
А что ты предлагаешь вместо ООП? Функциональный подход? Или ещё какие-то парадигмы, более успешные в решении обсуждаемой проблемы?
G>>А ты имеешь ввиду, что можно так спроектировать классы изначально, что потом она сама собой будет разрастаться только в "правильном" направлении? И это "правильное" направление (соблюдение SOLID и прочие принципы) как-бы настолько сильно и мощно заложено в изначальной архитектуре, что даже джуны и мидлы, которые пока не очень освоили SOLID, будут при доработке системы развивать её только в "правильном" направлении, без необходимости код ревью их кода?
IT>К этому нужно стремиться. В чём проблема?
IT>Если вернуться к моему изначальному комментарию, то я лишь утверждал, что необходимость следования данным принципам уже свидетельствует о проблемах в коде. Твой пример это только подтвердил. А как это решать — без понятия. У каждой конкретной проблемы в каждом конкретном случае своё конкретное решение.
Ясно. Ну с твоей точки зрения выходит, что любая методология будет иметь проблемы. Но ведь наличие проблем — это часть диалектической природы, от таких проблем нельзя уйти.
Или все же есть нечто более адекватное, чем ООП?
Здравствуйте, #John, Вы писали:
J>Здравствуйте,
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
J>"The code will be look much better if ..., what do you think?"
Можно использовать классический прием, принятый в западной культуре — сначала похвалить, потом покритиковать. При этом формулировка должна быть без давления, оставляя хотя бы видимость свободы выбора для автора кода.
Есть три уровня сводобы выбора:
1. Код хвалится, высказываются предложения по улучшению, но PR не аппрувится. В переводе с вежливого — это указания на то, что изменения должны быть сделаны.
2. То же самое, но код "approved with suggestions" — это значит: я хотел бы, чтобы изменения были внесены, но если тебе горит можно и так оставить.
3. Самый мягкий — код аппрувится, изменения полностью на усмотрение автора.
Вякие там could you и так далее не очень-то работают, если не оставляют хотя бы видимости свободы выбора. Да и не нужно отправлять такие многосложные конструкции коллегам, поменьше формальностей, так оно дружелюбнее выглядит. "what do you think" добавить в конце — хорошо, но вначале надо все-таки похвалить. Что-нибудь вроде "Looks fine to me, you may improve it a bit by ... though. What do you think?"
Здравствуйте, gyraboo, Вы писали:
G>А что ты предлагаешь вместо ООП? Функциональный подход? Или ещё какие-то парадигмы, более успешные в решении обсуждаемой проблемы?
Ты требуешь универсального решения всех проблем? У меня такого нет
G>Ясно. Ну с твоей точки зрения выходит, что любая методология будет иметь проблемы. Но ведь наличие проблем — это часть диалектической природы, от таких проблем нельзя уйти.
Любая методология, инструмент, паттерн, парадигма, принцип всегда привносит усложение в любое решение. Это аксиома. Вопрос лишь в том, будет ли и на сколько выигрышь превышать затраты.
G>Или все же есть нечто более адекватное, чем ООП?
Да причём здесь ООП. Давай без ООП и вообще без программирования.
Например, возьмём проблему превышения скорости транспортными средствами в жилых районах. В соответствии с принципом Лисковой нужно в каждый двор поставить по полицейскому (ревьюверу), который будет отлавливать нерадивых Вась Петиных. А можно решить проблему другим способом — положить лежачих полицейских и никаких стоячих ревьюверов тогда не понадобится.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>А что ты предлагаешь вместо ООП? Функциональный подход? Или ещё какие-то парадигмы, более успешные в решении обсуждаемой проблемы?
IT>Ты требуешь универсального решения всех проблем? У меня такого нет
G>>Ясно. Ну с твоей точки зрения выходит, что любая методология будет иметь проблемы. Но ведь наличие проблем — это часть диалектической природы, от таких проблем нельзя уйти.
IT>Любая методология, инструмент, паттерн, парадигма, принцип всегда привносит усложение в любое решение. Это аксиома. Вопрос лишь в том, будет ли и на сколько выигрышь превышать затраты.
Я уверен, что соблюдение SOLID, паттернов, элементов DDD, контрактное программирование, проверка инвариантов, практики Clean code дают выигрыш в затратах на сопровождение кода — т.к. багов меньше, код — надежнее. Кроме того, некоторые принципы, типа OCP, кроме прочих бенефитов снижают и кол-во регрессионных багов. Да и писать такой код проще, т.к. есть четкие правила, и ты не гоняешь "а как сделать? как назвать переменную? как спроектировать иерархию? писать или не писать проверку инвариантов? если писать, то полную или не полную? а писать ли тесты?", а следуешь этим правилам, указанным в обще-командном чек-листе и не паришся, как осел перед двумя идентичными стогами сена, а просто пишешь код. Также повышается уверенность в качестве кода и снижается страх перед неопределенностью — а взлетит или не взлетит? Если повезет и взлетит, достаточно ли хорошо и полно написан код?
G>>Или все же есть нечто более адекватное, чем ООП?
IT>Да причём здесь ООП. Давай без ООП и вообще без программирования.
IT>Например, возьмём проблему превышения скорости транспортными средствами в жилых районах. В соответствии с принципом Лисковой нужно в каждый двор поставить по полицейскому (ревьюверу), который будет отлавливать нерадивых Вась Петиных. А можно решить проблему другим способом — положить лежачих полицейских и никаких стоячих ревьюверов тогда не понадобится.
Ясно, ну на такой случай у нас в чек-листе есть правило — не заниматься оверинжинирингом. Не лепить везде OCP полиморфизм вместо условий, т.к. это не всегда нужно. Не оптимизировать раньше времени. И т.д.
SOLID ради SOLID — это тоже известная проблема, и те кто их практикует не дураки, не лепят их везде и всюду. Если есть компромисс, то идут на компромисс. SOLID применяется когда необходимость действительно назревает, т.е. делается рефакторинг. Единственное, что важно — это наличие тестов, которые защитят отрефакторенный код от регрессии, и с этим иногда беда и бывает.
Ну и прицнип KISS есть, хотя он не такой простой в реализации, как в формулировке. Делать просто — это сложная задача.
Здравствуйте, gyraboo, Вы писали:
G>Я уверен, что соблюдение SOLID, паттернов, элементов DDD, контрактное программирование, проверка инвариантов, практики Clean code дают выигрыш в затратах на сопровождение кода — т.к. багов меньше, код — надежнее.
давным давно я приводил пример, когда не работает SRP.
Проблема SRP (впрочем как и всех других принципов) в том, что они предписывают делать или не делать нечто, но не задают рамки своей применимости, оставляя это на откуп здравому смыслу. А с последним могут возникнуть проблемы, если желанию следовать какому-либо принципу присваивается наивысший приоритет.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>Я уверен, что соблюдение SOLID, паттернов, элементов DDD, контрактное программирование, проверка инвариантов, практики Clean code дают выигрыш в затратах на сопровождение кода — т.к. багов меньше, код — надежнее.
IT>Вот здесь
давным давно я приводил пример, когда не работает SRP.
Совершенно согласен с этой статьей, я её даже читал уже.
Хотя добавил бы, что код ревью часто все же немного упрощает сложность. Потому что на мой взгляд первичные коммиты по таску — это результат наслоения инженерного поиска, без упрощающего рефакторинга, т.к. во-первых у разраба глаз уже замылен его задачей, во-вторых, ему в лом без пинка что-то в коде упрощать, в-третьих, свежий взгляд со стороны часто видит излишнюю сложность кода, которую можно и нужно упростить, в том числе с помощью этих правил.
Ну и ещё часто невозможно собрать супер-team с мега-крутыми мозгами, в энтерпрайзе часто работают средние по уровню команды, и правила и дисциплинируют, и повышают качество, доводя его до приемлимо промышленного уровня, но не до того совершенства простоты, о котором ты говоришь.
IT>Проблема SRP (впрочем как и всех других принципов) в том, что они предписывают делать или не делать нечто, но не задают рамки своей применимости, оставляя это на откуп здравому смыслу. А с последним могут возникнуть проблемы, если желанию следовать какому-либо принципу присваивается наивысший приоритет.
Именно поэтому кроме SOLID в моем чек-листе (говорю только за себя), есть правила, "ограничивающие" их применение и остужающие пыл больных "паттерной лихорадкой" (хотя таких вроде уже почти не осталось, потому что сейчас каждый второй джун, а благодаря книгам Сиерры и Швеца и каждый второй заинтересованный школьник) знают про паттерны и SOLID. Это было откровением может только в начале 2000-х, когда все о них говорили но мало кто понимал что это такое и как это правильно применять, сейчас же — GRAPS/SOLID/паттерны — это просто часть всеобщей грамотности и единого мета-языка, а не Откровение Для Вчерашних Эникейщиков. Да, опыта в их реальном применении у начинающих коммерческих разрабов мало, но от зубов эта теория отскакивает будь здоров.
Упомянутые "ограничивающие" правила — это например KISS, напоминание про оверинжиниринг. Да и сами SOLID в моём чек-листе например стоят лишь на 40-м месте.
Как тут в соседней ветке про религиозные секты верно отметили, сейчас Великим Но Непонятным Чудом и Волшебной Таблеткой, Решающей Все Проблемы, является DDD, ну и наверное функциональщина с реактивщиной. Ну и конечно мантра "распилить монолит".
Здравствуйте, gyraboo, Вы писали:
G>Я уверен, что соблюдение SOLID, паттернов, элементов DDD, контрактное программирование, проверка инвариантов, практики Clean code дают выигрыш в затратах на сопровождение кода — т.к. багов меньше, код — надежнее. Кроме того, некоторые принципы, типа OCP, кроме прочих бенефитов снижают и кол-во регрессионных багов. Да и писать такой код проще, т.к. есть четкие правила, и ты не гоняешь "а как сделать? как назвать переменную? как спроектировать иерархию? писать или не писать проверку инвариантов? если писать, то полную или не полную? а писать ли тесты?", а следуешь этим правилам, указанным в обще-командном чек-листе и не паришся, как осел перед двумя идентичными стогами сена, а просто пишешь код. Также повышается уверенность в качестве кода и снижается страх перед неопределенностью — а взлетит или не взлетит? Если повезет и взлетит, достаточно ли хорошо и полно написан код?
IMHO принцип Лискова- элементарно обобщение рациональности. Если до чела не доходит, что у него дизайн классов это какой-то понос, можно рубануть принципом лискова, и закончить спор в свою пользу.
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Сильно зависит от компании, и от твоей в ней позиции.
В идеале со ссылками на guidelines — "здесь мы используем исключения, а не return codes", и т.п..
Здравствуйте, SkyDance, Вы писали:
SD>Сильно зависит от компании, и от твоей в ней позиции.
То есть джуны и мидлы к код ревью не подпускаются? Чтобы не дай бог не нашли баг в коде помидора?
SD>В идеале со ссылками на guidelines — "здесь мы используем исключения, а не return codes", и т.п..
+1, хороший подход, спорить с докой врятли кто будет, ничего личного вообщем
Здравствуйте, Antidote, Вы писали:
A>Здравствуйте, SkyDance, Вы писали:
SD>>Сильно зависит от компании, и от твоей в ней позиции.
A>То есть джуны и мидлы к код ревью не подпускаются? Чтобы не дай бог не нашли баг в коде помидора?
Да пускают наверное, просто надо же понять сколько раз Ку делать, а то вдруг мало и потом эцилопп ночью изобьет?
Здравствуйте, Тёмчик, Вы писали:
Тё>IMHO принцип Лискова- элементарно обобщение рациональности. Если до чела не доходит, что у него дизайн классов это какой-то понос, можно рубануть принципом лискова, и закончить спор в свою пользу.
Совершенно точно, но как и любое обобщение, ему не хватает конкретики, поэтому увязать в голове обобщенный принцип и свою конкретную ситуацию в своем конкретном коде бывает тяжело, особенно как нет практики, а в описании принципа нет ни конкретики (на взгляд начинающего программиста-практика), ни набора хороших примеров его нарушений и соблюдений:
Пусть q(x) является свойством, верным относительно объектов x некоторого типа T. Тогда q(y) также должно быть верным для объектов y типа S, где S является подтипом типа T.
Охрененно понятная и практичная формулировка, просто верх рациональности))
Роберт Мартин конечно как всегда приходит на выручку, но его формулировка тоже не верх практичности.
J>как вежливо делать замечания по коду, желательно с примерами на англ.
Вообще, всем по большому счету пофигу на эту вежливость. Главное не цепляйся к мелочам (я конечно понимаю, что для одного это мелочь а для другого смысл всей жизни, но все-таки...), если критикуешь, предлагай, как сделать лучше.
Ну и еще, старайся находить реальные проблемы, а не высасывать их из пальца, и будет тебе счастье.
В обоих случаях, важно не приказывать и не требовать, надо предлагать как сделать лучше, ну и вообще круто, если это неочевидно объяснять почему так лучше.
SD>>Сильно зависит от компании, и от твоей в ней позиции. A>То есть джуны и мидлы к код ревью не подпускаются? Чтобы не дай бог не нашли баг в коде помидора?
Позиция определяет уровень комментариев. На определенных позициях комментарии будут "зачем вообще этот дифф выслан" или "предоставьте доку, что вы вообще делаете такое".
На других — "это неканонично для языка, писать if length(Var) == length("false")."
А баги бывают в любом коде, даже у помидоров. Просто чем помидорнее, тем баги забористее.
Здравствуйте, IT, Вы писали:
IT>Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования.
Да в принципе, стоит только переопределить неабстрактный метод, и получаем нарушение LSP.
Здравствуйте, SkyDance, Вы писали:
SD>>>Сильно зависит от компании, и от твоей в ней позиции. A>>То есть джуны и мидлы к код ревью не подпускаются? Чтобы не дай бог не нашли баг в коде помидора?
SD>Позиция определяет уровень комментариев. На определенных позициях комментарии будут "зачем вообще этот дифф выслан" или "предоставьте доку, что вы вообще делаете такое". SD>На других — "это неканонично для языка, писать if length(Var) == length("false")."
SD>А баги бывают в любом коде, даже у помидоров. Просто чем помидорнее, тем баги забористее.
Все как раз наоборот.
Чем помидорнее, тем проще все должно искаться и правиться, т.к. есть навык написания робастного (устойчивого) кода.
А вот джуны могут так накосячить, что помидорам за ними месяцами не разобраться.
sts>Чем помидорнее, тем проще все должно искаться и правиться, т.к. есть навык написания робастного (устойчивого) кода.
Именно поэтому если в коде таки есть баг, он очень, очень ОЧЕНЬ неочевиден и требует нетривиальных умений.
Иными словами, чтобы найти (и тем более исправить) баг в коде сеньора, нужен... еще более сеньор.
а "Is this ... done?" вопрос уровня "наставник ученику", т.е. корректен если ты какой лид, или сеньёр ревьювящий за джуном. А для равных между собой он используется только когда вы уже враги которые друг друга валят при первой возможности. Злорадное "вот ты и попался! вот тут нот дан!".
Если нам не помогут, то мы тоже никого не пощадим.
