514

Ответ на пост «Я написал свою книгу по программированию»3

Плохо, прям очень плохо. Надеюсь всё плюсы и положительные отзывы этому посту это по большей части аналог с али экспрессовским "товар получил, не открывал, ставлю 5 звёзд" и реакция на БЕСПЛАТНО. Даже просто открыв этот pdf можно увидеть на сколько автору было безразлично удобство чтения этого опуса. Микро формат страниц, который просто убивает форматирование кода, сразу бросается в глаза. Но в конце концов вёрстку можно исправить, если автору есть что добавить в довольно пропаханную тему базовых знаний по одному из достаточно старых ООП языков. Я как практикующий товарищ решил сразу посмотреть какой то более менее цельный и минимально содержательный фрагмент кода, так как разделяю мнение что хороший код является "само документируемым" и также может показать общий уровень книги. Первый такой фрагмент нашёлся на 119 странице и содержал очень плодотворную тему рефакторинга кода. Вообще, эта тема обсуждается начиная с банальных уроков программирования в школе, где вас просят хотя бы давать осмысленные имена переменным и проходит через весь опыт практического программирования, где является одним из ключевых элементов борьбы со сложностью. Самое сложное тут суметь уместить в маленьком примере какую то идею, чтобы читатель смог её увидеть, а не просто "поверить автору". И даже в сравнительно больших фрагментах программ с подробным разбором на протяжении всей книги (например "Чистый Код" Роберта Мартина) бывает сложно это реализовать и люди приходя на проект в 1 миллион строк сталкиваются с тем, что рефакторинг в рафинированных примерах и реальном проекте может значительно отличаться по сложности реализации. Это я увлёкся лирикой, перейдём к коду, у нас есть некоторый метод TryOpenDoor со следующей сигнатурой:

private static bool TryOpenDoor(); (посмотреть реализацию можно на странице 119)

и после ряда "улучшений" и вынесения методов получается следующий код:

Ответ на пост «Я написал свою книгу по программированию» Программирование, IT, Айтишники, Программист, Csharp, Текст, Ответ на пост, Длиннопост

Давай посмотрим что с ним не так? Пункты будут идти по моему субъективному убыванию критичности.

1) Это другое поведение ! Это прям вообще не нормальная вещь. И дело не в том что мы переименовали метод, это как раз допустимо. Мы изменили сигнатуру метода, ранее он возвращал булевское значение (правда\ложь) , теперь он ничего не возвращает. В случае реального рефакторинга IDE нам бы подсказала и такой проект просто не собрался бы. Почему так получилось ? Потому что по факту начальный код и конечный выполняет немного разные вещи, и оригинальный метод является частью InteractWithDoor() и он естественным образом разваливается на 2 метода, которые и хочется объединить под новым более общим методом.

2) Смешение уровней абстракции. Это может показаться не критичным на таком маленьком примере, но в реальности это огромная проблема и очень важно на начальном этапе дать правильное понимание базовых вещей в архитектуре кода. Так как когда вы перейдёте от 20 строчных примеров к проектам с 20 000 классов вы сможете намного ухудшить качество своего кода, но не улучшить его. У нас есть метод запроса\чтения возраста из консоли, если кто не знает это ReadInt, и первый вывод консоли логически относится к этому методу, тем более в нём уже есть интерактивность с пользователем. На том уровне где мы оперируем методом ReadInt aka GetAge как правильно не должно быть вывода в консоль, если он есть внутри ReadInt.

3) ReadInt() - это вызывает вопросы. Для начала сообщу что почти любая IDE выведет тип возвращаемого значения просто при наведение на метод. В старом коде на си, например, можно встретить обозначение типов в приватных переменных класса, но даже в таком случае оно дополняет название, а не заменяет его. Если бы метод хотя бы назывался GetAgeInt я бы не стал придираться, в конце концов есть принятые в командах стандарты и практики ,а также вкусовщина. Можно возразить "но этот же метод действительно просто получает Int32 из консоли", и с этим можно было бы согласиться, если бы это был какой то публичный метод для consol-и, но даже в таком случае ключевым тут было бы что это значение из консоли. То есть выглядеть должно было, либо так ConsoleEx.GetInt32(), либо GetInt32FromConsole(). Приватный метод, особенно с таким маленьким скоупом, должен иметь очень специфичное функции имя.

4) В книге автор заявляет что он добился успеха выделив "чистое правило" открытия двери в TryOpenDoor() , кстати это было названием оригинального метода. Но давайте посмотрим на этот метод, что в этой строке "age >= 18" есть о двери ??? У меня нейминг вызывает вопросы

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

Многие в комментариях гонятся за какими то "актуальными знаниями", которые толи должны быть в книге, толи не могут быть в ней т.к. она устаревает. Но в реальности я легко могу сказать тимлиду, что не знаю что то требуемое из нового реквайремента или таски и мне нужно время на изучение документации\кода. Но я плохо себе представляю как бы я смог объяснить такое качество кода, при том что в нём самый базовый функционал, который будет работать и на версиях языка десятилетней давности, такой код плох даже для intern позиции.

Как мог бы выглядеть этот код с моей точки зрения. Если мы хотим реализовать InteractWithDoor, то исходный метод, конечно, недостаточен и он разбивается на 2 составляющие, получение возраста и его валидация, но далее нам требуется открытие\отказ в открытие двери, что в коде автора реализовано через сообщение "Дверь не для тебя!" с очередным нарушением уровня абстракции.

Ответ на пост «Я написал свою книгу по программированию» Программирование, IT, Айтишники, Программист, Csharp, Текст, Ответ на пост, Длиннопост

Пара комментариев по коду.

1) Конечно, метод EnableRussianSymbolsForConsole не относится к InteractWithDoor, но когда книжный пример при копирование в IDE на некоторых системах будет выдавать тебе вопросы вместо текста это не очень хорошо. И для примера добавить такой вызов допустимо, но лично я бы предпочёл просто использовать английский во всех запросах.

2) Длинный нейминг методов допустим и даже хорош для само документирования кода, в случае если это внутренние методы с малым скоупом видимости (в данном случае все методы кроме InteractWithDoor являются приватными).

3) Может показаться что методы ShowNotAllowedMessage и OpenDoor немного избыточны т.к. у них однострочная реализация, но лично моя практика показывает что такая разбивка оправдана.

4) Код получился длиннее и это нормально, особенно когда речь идёт про рефакторинг таких маленьких фрагментов. Главный показатель качества кода не его количество, а его читаемость и простота модификации. При рефакторинге больших объёмов кода часто бывает и обратный эффект из-за устранение дублирования и избыточной логики, вызванной кривой архитектурой.

Хороший код на верхнем уровне читается почти как осмысленный текст без матана и всяких сложных условий.

Взять возраст из консоли и используя его попробовать открыть дверь

Что же такое попробовать открыть дверь?

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

не идеально, согласен, но я на многое и не претендую, а теперь попробуйте прочитать оригинальную программу автора.

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

Я смог - сможешь и ты!

2.2K поста6.7K подписчика

Правила сообщества

Нельзя:

- оскорблять;

- использовать нецензурную лексику;

- обесценивать чужие достижения, даже если для вас они незначительны.

14
Автор поста оценил этот комментарий
Сравнение с false/true булевых выражений тоже выдаёт новичка.
раскрыть ветку (1)
4
Автор поста оценил этот комментарий

Да, тоже обратил внимание, очень характерный признак, но не стал выносить т.к. формально отнёс к граничным случаем субъективщины\привычки\стилистики.

49
Автор поста оценил этот комментарий

А мне нравятся программисты (поскольку сам такой). Люди удивительно высокого самомнения (в основном). Снобизм зашкаливает. Главный лозунг - "Все пидарасы, а я один д'Артаньян!". Самое главное обосрать чужой код, начиная с нейминга (кстати, это черта профессионала). И в долгих дискуссиях доказывать что лучше, борщ или щи (яйца полощи). Обязательно нужно деклассировать визави, растоптать, обоссать и выкинуть. За это я вас люблю, коллеги.

раскрыть ветку (1)
3
Автор поста оценил этот комментарий

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

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

Примерно 80-90% методов в реальных проектах имеют только одно место вызова. Методы разбиваются с точки зрения выделения логических единиц исполнения (одной функции) на данном уровне абстракции. И в будущем это , конечно, позволяет их переиспользовать, но нет смысла закладываться на будущее, это сделает код только сложнее, достаточно сделать логичную структуру "без костылей" и при необходимости её будет не сложно модифицировать под текущие требования.

11
Автор поста оценил этот комментарий

Да там и с точки зрения просто вёрстки и русского языка тоже огонь

Иллюстрация к комментарию
Иллюстрация к комментарию
раскрыть ветку (1)
2
Автор поста оценил этот комментарий

Да там жесть. как будто пытались нахреначить по больше страниц.

7
Автор поста оценил этот комментарий

Ну ты не лучше написал. Дочитал до того где ты усираешься со смены сигнатуры метода при рефакторинге и дальше смысла нет. Адаптеры изобрели не для тебя походу. Сигнатуры при рефакторинге меняют если это надо. Если сложно остальную часть системы менять - делаешь под нее адаптер и все.

раскрыть ветку (1)
1
Автор поста оценил этот комментарий

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

показать ответы
145
Автор поста оценил этот комментарий

Шо то хуйня, шо эта хуйня шо обе эти хуйни и т.д.


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


За каким хукм вообще выносить обычную проверку age >= 18 в отдельный приватный метод. Private говорит о том, что этот метод не планируется изменятся в дочернем классе. Есть условие на текущий момент age >=18, все! Это задача бизнес логики работы приложения, будет ли она меняется - хуй его знает. Может да, а может нет. Дак зачем сейчас из одной строчки кода if age >= 18 плодить целый вызов метода, который будет жить ещё н-лет.

Уж лучше бы оба автора запихали настройку для age=18 в сам класс. Потому что это единственная величина которая может поменяться и ее выгодно хранить в конфигурации чтобы не править код по сто раз.


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


За ким хером настройки консоли про кодировку ушли в класс который открывает двери. Разделяй ответственность! Классу открытия дверей похуй на консоль. Если так любишь абстракции дай классу интерфейс пусть он туда срет свой вывод.

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


Зачем вывод ошибки в консоль выносить в отдельный метод? Что ты планируешь там писать, выбор языка? А захуй там выбор языка? Если уж и будем мултиланг то это будет какая-нибудь обертка в один вызов, а это значит что в 99.9 процентов ты ради одной строчки создал отдельный, нахуй не кому ненужный метод.

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


По стилистике ещё пиздану: не надо убирать фигурные скобки в if else, код от этого не становится легче. То что ты убрал пару строк не делает код читаемым. Глазу приходится искать границы. В некоторых языках, есть даже стандарт оформления, где за отсутствие скобок пиздят палкой по рукам.


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


Давай возьмём пример который вы обсасываете и делаете его "лучше". На самом деле, в этом коде, главное говно - это функция ReadInt.

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

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

раскрыть ветку (1)
1
Автор поста оценил этот комментарий

Тут нет никакого класса, потому что в книге это пример дан до описания классов и я это учитывал когда писал логику, так как в случае класса door код выглядел совсем бы по другому.

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

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

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

По поводу нужен ли рефакторинг тут. В некой виртуальном продукте этот код мог бы оставаться в изначальном виде хоть десятки лет и нормально работать, но это был БУКВАЛЬНО ПРИМЕР РЕФАКТОРИНГА ОТ АВТОРА. Поэтому претензия не ясна.

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

показать ответы
0
Автор поста оценил этот комментарий

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

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


С другой стороны, из функции можно вызывать какую-нибудь SendToLog("не пущу"), которая параметр перекидывает в WriteLine. Но назначение такой функции -- исключительно отладочное, чтобы можно было понимать, как функция отработала внутри. Это позволяет весь поток логов в какой-то момент перенаправить в файл или куда-то ещё.


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


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

Иллюстрация к комментарию
раскрыть ветку (1)
0
Автор поста оценил этот комментарий

Согласен

0
Автор поста оценил этот комментарий

Вот это на первой картинке - конечно умная и рабочая (и возможно даже типовая, не первый раз вижу подобное) конструкция, но на мой взгляд нахрен такое делать не надо, особенно в качестве примера - одна строка делает сразу несколько вещей, и тело цикла - код, который в идеале НЕ должен выполняться. Это всё понижает читаемость кода "беглым взглядом".

Это не мой родной язык, и возможно, конечно, в сишарпе так принято, но я бы делал что-то вроде второй картинки (особенно если сишарп умеет оптимизировать такие вызовы без рекурсии, хз как по-русски зовется tail call optimization). И вызывал бы уже этот метод из функции запроса возраста.


И даже этот результат мне не нравится и я бы разделил условие на две строки как на третьей картинке. Теперь всё читается без знания используемых функций, всё задокументировано в одном месте, джун не потеряется :)

Иллюстрация к комментарию
Иллюстрация к комментарию
Иллюстрация к комментарию
раскрыть ветку (1)
0
Автор поста оценил этот комментарий

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


*насчёт рекурсии в книге не уверен не заметил где она вводится, смотрел по диагонали, но вроде как не было до этого примера.

показать ответы