четверг, 5 августа 2021 г.

[prog] Хорошая иллюстрация кода, про который остается разве что сказать "мне кажется, что можно сделать проще"

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

К сожалению, такие воспоминания сложно иллюстрировать примерами кода.

Но вот на Хабре нашел статью с примером, который, как мне думается, ну просто идеален для подобной иллюстрации: История одного фееричного провала тестового задания на C#. Код, который написал автор упомянутой статьи, выложен на GitHub, там его и можно посмотреть. В частности, мне хватило беглого взгляда на вот этот файл.

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

Если говорить о содержимом класса TestApp::Schedule, то у меня лично сходу следующие претензии к уведенному:

  • нет описания принципа хранения расписания в TestApp::Schedule. В классе есть набор членов, но нет описания как и для чего они используются, каким образом расписание представляется посредством этих членов. Это значит, что тому, кто будет сопровождать такой класс, придется всю логику выколупывать из кода. И надеятся, что исходная логика была этим самым кодом выражена правильно, что не далеко не факт, как показывает мой многолетний опыт;
  • слишком большой объем некоторых методов. В частности конструктора Schedule. Ладно бы эти 120 строк были сгенерированы автоматически, но ведь они написаны вручную;
  • слишком большая вложенность if-ов в некоторых местах. Опять же, если бы это был автоматически сгенерированный код, то ладно. Но это же написано вручную. Простите мне мой французский, но когда программист пишет подобную вложенность if-ов, то это наводит на подозрения об ошибках в ДНК. В мое время человеку бы могли запросто сказать, что он ошибся профессией.

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

Так что я бы лично такую реализацию тестового задания бы завернул. Может быть с более лояльной общей оценкой :)

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

Комментариев нет: